From 0bceb6e19ff97e9318f6bd8eab363f9eb0dc5c25 Mon Sep 17 00:00:00 2001 From: Mikki Weesenaar Date: Fri, 13 May 2022 13:26:29 +0200 Subject: [PATCH 01/24] Enforcing a redirect to setup of otp device when none available for user --- tests/test_views_login.py | 14 ++--- tests/test_views_mixins.py | 53 ++++++++++++++++++- .../two_factor/core/setup_complete.html | 4 ++ two_factor/views/core.py | 10 +++- 4 files changed, 72 insertions(+), 9 deletions(-) diff --git a/tests/test_views_login.py b/tests/test_views_login.py index 92ec5c3c6..5e7f214ed 100644 --- a/tests/test_views_login.py +++ b/tests/test_views_login.py @@ -38,7 +38,7 @@ def test_valid_login(self, mock_signal): response = self._post({'auth-username': 'bouke@example.com', 'auth-password': 'secret', 'login_view-current_step': 'auth'}) - self.assertRedirects(response, resolve_url(settings.LOGIN_REDIRECT_URL)) + self.assertRedirects(response, reverse('two_factor:setup')) # No signal should be fired for non-verified user logins. self.assertFalse(mock_signal.called) @@ -80,7 +80,8 @@ def test_valid_login_with_allowed_external_redirect(self): {'auth-username': 'bouke@example.com', 'auth-password': 'secret', 'login_view-current_step': 'auth'}) - self.assertRedirects(response, redirect_url, fetch_redirect_response=False) + self.assertEqual(self.client.session.get('next'), redirect_url) + self.assertRedirects(response, reverse('two_factor:setup'), fetch_redirect_response=False) def test_valid_login_with_disallowed_external_redirect(self): redirect_url = 'https://test.disallowed-success-url.com' @@ -90,7 +91,7 @@ def test_valid_login_with_disallowed_external_redirect(self): {'auth-username': 'bouke@example.com', 'auth-password': 'secret', 'login_view-current_step': 'auth'}) - self.assertRedirects(response, reverse('two_factor:profile'), fetch_redirect_response=False) + self.assertRedirects(response, reverse('two_factor:setup'), fetch_redirect_response=False) @mock.patch('two_factor.views.core.time') def test_valid_login_primary_key_stored(self, mock_time): @@ -395,12 +396,12 @@ def test_login_different_user_on_existing_session(self, mock_logger): response = self._post({'auth-username': 'bouke@example.com', 'auth-password': 'secret', 'login_view-current_step': 'auth'}) - self.assertRedirects(response, resolve_url(settings.LOGIN_REDIRECT_URL)) + self.assertRedirects(response, reverse('two_factor:setup')) response = self._post({'auth-username': 'vedran@example.com', 'auth-password': 'secret', 'login_view-current_step': 'auth'}) - self.assertRedirects(response, resolve_url(settings.LOGIN_REDIRECT_URL)) + self.assertRedirects(response, reverse('two_factor:setup')) def test_missing_management_data(self): # missing management data @@ -431,8 +432,7 @@ def test_login_different_user_with_otp_on_existing_session(self): response = self._post({'auth-username': 'bouke@example.com', 'auth-password': 'secret', 'login_view-current_step': 'auth'}) - self.assertRedirects(response, - resolve_url(settings.LOGIN_REDIRECT_URL)) + self.assertRedirects(response, reverse('two_factor:setup')) response = self._post({'auth-username': 'vedran@example.com', 'auth-password': 'secret', diff --git a/tests/test_views_mixins.py b/tests/test_views_mixins.py index a5fae597b..a0f26e099 100644 --- a/tests/test_views_mixins.py +++ b/tests/test_views_mixins.py @@ -1,8 +1,12 @@ +from binascii import unhexlify + from django.conf import settings from django.shortcuts import resolve_url from django.test import TestCase +from django.urls import reverse +from django_otp.oath import totp -from .utils import UserMixin +from .utils import UserMixin, method_registry class OTPRequiredMixinTest(UserMixin, TestCase): @@ -53,3 +57,50 @@ def test_verified(self): self.login_user() response = self.client.get('/secure/') self.assertEqual(response.status_code, 200) + + @method_registry(['generator']) + def test_valid_login_with_redirect_field_name_without_device(self): + self.create_user() + protected_url = '/secure/' + + # Open URL that is protected + # assert go to login page + # assert the next param not in the session (but still in url) + response = self.client.get(protected_url) + redirect_url = "%s?%s%s" % (reverse('two_factor:login'), 'next=', protected_url) + self.assertRedirects(response, redirect_url) + self.assertEqual(self.client.session.get('next'), None) + + # Log in given the last redirect + # assert redirect to setup + response = self.client.post( + redirect_url, + {'auth-username': 'bouke@example.com', + 'auth-password': 'secret', + 'login_view-current_step': 'auth'}) + self.assertRedirects(response, reverse('two_factor:setup')) + self.assertEqual(self.client.session.get('next'), protected_url) + + # Setup the device accordingly + # assert redirect to setup completed + # assert button for redirection to the original page + response = self.client.post( + reverse('two_factor:setup'), + data={'setup_view-current_step': 'welcome'}) + self.assertContains(response, 'Token:') + self.assertContains(response, 'autofocus="autofocus"') + self.assertContains(response, 'inputmode="numeric"') + self.assertContains(response, 'autocomplete="one-time-code"') + + key = response.context_data['keys'].get('generator') + bin_key = unhexlify(key.encode()) + response = self.client.post( + reverse('two_factor:setup'), + data={'setup_view-current_step': 'generator', + 'generator-token': totp(bin_key)}, + follow=True, + ) + + self.assertRedirects(response, reverse('two_factor:setup_complete')) + self.assertContains(response, 'Continue where I left off') + self.assertContains(response, '{% trans "Add Phone Number" %}

{% endif %} + {% if next %} + {% trans "Continue where I left off" %} + {% endif %} {% endblock %} diff --git a/two_factor/views/core.py b/two_factor/views/core.py index 5e1e9b0b5..da6f7ae31 100644 --- a/two_factor/views/core.py +++ b/two_factor/views/core.py @@ -182,7 +182,13 @@ def done(self, form_list, **kwargs): samesite=getattr(settings, 'TWO_FACTOR_REMEMBER_COOKIE_SAMESITE', 'Lax'), ) - return response + return response + + # If the user does not have a device. + else: + if self.request.GET.get('next'): + self.request.session['next'] = redirect_to + return redirect('two_factor:setup') # Copied from django.conrib.auth.views.LoginView (Branch: stable/1.11.x) # https://github.com/django/django/blob/58df8aa40fe88f753ba79e091a52f236246260b3/django/contrib/auth/views.py#L63 @@ -608,8 +614,10 @@ class SetupCompleteView(TemplateView): template_name = 'two_factor/core/setup_complete.html' def get_context_data(self): + redirect_url = self.request.session.pop('next', None) return { 'phone_methods': get_available_phone_methods(), + 'next': redirect_url } From 48d1ae0f29a93a22c3afd689ea5a0a02030b66dd Mon Sep 17 00:00:00 2001 From: Mikki Weesenaar Date: Mon, 30 May 2022 22:54:47 +0200 Subject: [PATCH 02/24] Rework after feedback. --- tests/test_views_mixins.py | 4 +-- .../two_factor/core/setup_complete.html | 4 --- two_factor/views/core.py | 36 +++++++++++++++---- 3 files changed, 31 insertions(+), 13 deletions(-) diff --git a/tests/test_views_mixins.py b/tests/test_views_mixins.py index a0f26e099..dfbfe9595 100644 --- a/tests/test_views_mixins.py +++ b/tests/test_views_mixins.py @@ -101,6 +101,4 @@ def test_valid_login_with_redirect_field_name_without_device(self): follow=True, ) - self.assertRedirects(response, reverse('two_factor:setup_complete')) - self.assertContains(response, 'Continue where I left off') - self.assertContains(response, '{% trans "Add Phone Number" %}

{% endif %} - {% if next %} - {% trans "Continue where I left off" %} - {% endif %} {% endblock %} diff --git a/two_factor/views/core.py b/two_factor/views/core.py index da6f7ae31..e46cca90a 100644 --- a/two_factor/views/core.py +++ b/two_factor/views/core.py @@ -187,7 +187,7 @@ def done(self, form_list, **kwargs): # If the user does not have a device. else: if self.request.GET.get('next'): - self.request.session['next'] = redirect_to + self.request.session['next'] = self.get_success_url() return redirect('two_factor:setup') # Copied from django.conrib.auth.views.LoginView (Branch: stable/1.11.x) @@ -400,7 +400,7 @@ def dispatch(self, request, *args, **kwargs): @class_view_decorator(never_cache) @class_view_decorator(login_required) -class SetupView(IdempotentSessionWizardView): +class SetupView(SuccessURLAllowedHostsMixin, IdempotentSessionWizardView): """ View for handling OTP setup using a wizard. @@ -423,6 +423,27 @@ class SetupView(IdempotentSessionWizardView): # Other forms are dynamically added in get_form_list() ) + # Copied from django.conrib.auth.views.LoginView (Branch: stable/1.11.x) + # https://github.com/django/django/blob/58df8aa40fe88f753ba79e091a52f236246260b3/django/contrib/auth/views.py#L63 + def get_success_url(self): + url = self.get_redirect_url() + return url or reverse('two_factor:setup_complete') + + # Copied from django.conrib.auth.views.LoginView (Branch: stable/1.11.x) + # https://github.com/django/django/blob/58df8aa40fe88f753ba79e091a52f236246260b3/django/contrib/auth/views.py#L67 + def get_redirect_url(self): + """Return the user-originating redirect URL if it's safe.""" + redirect_to = self.request.POST.get( + REDIRECT_FIELD_NAME, + self.request.GET.get(REDIRECT_FIELD_NAME, '') + ) + url_is_safe = url_has_allowed_host_and_scheme( + url=redirect_to, + allowed_hosts=self.get_success_url_allowed_hosts(), + require_https=self.request.is_secure(), + ) + return redirect_to if url_is_safe else '' + def get_method(self): method_data = self.storage.validated_step_data.get('method', {}) method_key = method_data.get('method', None) @@ -433,7 +454,7 @@ def get(self, request, *args, **kwargs): Start the setup wizard. Redirect if already enabled. """ if default_device(self.request.user): - return redirect(self.success_url) + return redirect(self.get_success_url()) return super().get(request, *args, **kwargs) def get_form(self, step=None, **kwargs): @@ -501,7 +522,7 @@ def done(self, form_list, **kwargs): raise NotImplementedError("Unknown method '%s'" % method.code) django_otp.login(self.request, device) - return redirect(self.success_url) + return redirect(self.get_success_url()) def get_form_kwargs(self, step=None): kwargs = {} @@ -613,11 +634,14 @@ class SetupCompleteView(TemplateView): """ template_name = 'two_factor/core/setup_complete.html' + def get(self, request, *args, **kwargs): + if request.session.get('next'): + return redirect(request.session.get('next')) + return super().get(request, *args, **kwargs) + def get_context_data(self): - redirect_url = self.request.session.pop('next', None) return { 'phone_methods': get_available_phone_methods(), - 'next': redirect_url } From ee0d2b6c1c0358d8bea9522865507d4fca7259c9 Mon Sep 17 00:00:00 2001 From: Darrel O'Pry Date: Thu, 23 Jun 2022 08:22:42 -0400 Subject: [PATCH 03/24] fix: replace use of SuccessURLAllowedHostsMixin with RedirectUrlMixin --- two_factor/views/core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/two_factor/views/core.py b/two_factor/views/core.py index e46cca90a..def045333 100644 --- a/two_factor/views/core.py +++ b/two_factor/views/core.py @@ -400,7 +400,7 @@ def dispatch(self, request, *args, **kwargs): @class_view_decorator(never_cache) @class_view_decorator(login_required) -class SetupView(SuccessURLAllowedHostsMixin, IdempotentSessionWizardView): +class SetupView(RedirectURLMixin, IdempotentSessionWizardView): """ View for handling OTP setup using a wizard. From 6144f37a2f6f8a8b1e34963aed451294a77fb9e6 Mon Sep 17 00:00:00 2001 From: Darrel O'Pry Date: Thu, 23 Jun 2022 08:40:42 -0400 Subject: [PATCH 04/24] chore: Add Changelog entry --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 35838b852..7cde32a7d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +## Unreleased + +### Added +- Enforcing a redirect to setup of otp device when none available for user [#550](https://github.com/jazzband/django-two-factor-auth/pull/500) + ## 1.14.0 ### Added From a2125df2f7598dd0304649c0eb3f2e5cd53f206c Mon Sep 17 00:00:00 2001 From: Darrel O'Pry Date: Thu, 7 Jul 2022 10:51:57 -0400 Subject: [PATCH 05/24] feat: drop django 2.2, 3.0, and 3.1 support (#497) --- CHANGELOG.md | 5 +++++ README.rst | 4 ++-- docs/requirements.rst | 3 +-- setup.py | 5 +---- tox.ini | 7 ------- two_factor/admin.py | 8 +------- two_factor/views/core.py | 9 +-------- 7 files changed, 11 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7cde32a7d..c6d1d6a3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,13 @@ +# Changelog + ## Unreleased ### Added - Enforcing a redirect to setup of otp device when none available for user [#550](https://github.com/jazzband/django-two-factor-auth/pull/500) +### Removed +- Django 2.2, 3.0, and 3.1 support + ## 1.14.0 ### Added diff --git a/README.rst b/README.rst index 0acc9a2ed..e7ba29a43 100644 --- a/README.rst +++ b/README.rst @@ -37,8 +37,8 @@ providing Django sessions with a foreign key to the user. Although the package is optional, it improves account security control over ``django.contrib.sessions``. -Compatible with modern Django versions. At the moment of writing that's -including 2.2, 3.1, 3.2, and 4.0 on Python 3.7, 3.8, 3.9 and 3.10. +Compatible with supported Django and Python versions. At the moment of writing that +includes 3.2 and 4.0 on Python 3.7, 3.8, 3.9 and 3.10. Documentation is available at `readthedocs.org`_. diff --git a/docs/requirements.rst b/docs/requirements.rst index 9a0672df7..5ffd90d10 100644 --- a/docs/requirements.rst +++ b/docs/requirements.rst @@ -3,8 +3,7 @@ Requirements Django ------ -Modern Django versions are supported. Currently this list includes Django 2.2, -3.0, 3.1, 3.2, and 4.0. +Supported Django versions are supported. Currently this list includes Django 3.2, and 4.0. Python ------ diff --git a/setup.py b/setup.py index eac36b430..8985c3ac3 100644 --- a/setup.py +++ b/setup.py @@ -12,7 +12,7 @@ license='MIT', packages=find_packages(exclude=('example', 'tests')), install_requires=[ - 'Django>=2.2', + 'Django>=3.2', 'django_otp>=0.8.0', 'qrcode>=4.0.0,<7.99', 'django-phonenumber-field>=1.1.0,<7', @@ -30,9 +30,6 @@ 'Development Status :: 5 - Production/Stable', 'Environment :: Web Environment', 'Framework :: Django', - 'Framework :: Django :: 2.2', - 'Framework :: Django :: 3.0', - 'Framework :: Django :: 3.1', 'Framework :: Django :: 3.2', 'Framework :: Django :: 4.0', 'Intended Audience :: Developers', diff --git a/tox.ini b/tox.ini index a926f9952..1bacdb87c 100644 --- a/tox.ini +++ b/tox.ini @@ -2,8 +2,6 @@ ; Minimum version of Tox minversion = 1.8 envlist = - py{37,38,39}-dj22-{normal,yubikey,custom_user}, - py{37,38,39}-dj31-{normal,yubikey,custom_user}, py{37,38,39,310}-dj32-{normal,yubikey,custom_user}, py{38,39,310}-dj40-{normal,yubikey,custom_user} py{38,39,310}-djmain-{normal,yubikey,custom_user} @@ -17,9 +15,6 @@ python = [gh-actions:env] DJANGO = - 2.2: dj22 - 3.0: dj30 - 3.1: dj31 3.2: dj32 4.0: dj40 main: djmain @@ -39,8 +34,6 @@ basepython = py39: python3.9 py310: python3.10 deps = - dj22: Django<2.3 - dj31: Django<3.2 dj32: Django<4.0 dj40: Django<4.1 djmain: https://github.com/django/django/archive/main.tar.gz diff --git a/two_factor/admin.py b/two_factor/admin.py index 7d9b29513..7957832f6 100644 --- a/two_factor/admin.py +++ b/two_factor/admin.py @@ -3,16 +3,10 @@ from django.contrib.auth import REDIRECT_FIELD_NAME from django.contrib.auth.views import redirect_to_login from django.shortcuts import resolve_url +from django.utils.http import url_has_allowed_host_and_scheme from .utils import monkeypatch_method -try: - from django.utils.http import url_has_allowed_host_and_scheme -except ImportError: - from django.utils.http import ( - is_safe_url as url_has_allowed_host_and_scheme, - ) - class AdminSiteOTPRequiredMixin: """ diff --git a/two_factor/views/core.py b/two_factor/views/core.py index def045333..339b1d886 100644 --- a/two_factor/views/core.py +++ b/two_factor/views/core.py @@ -20,6 +20,7 @@ from django.urls import reverse from django.utils.decorators import method_decorator from django.utils.functional import cached_property +from django.utils.http import url_has_allowed_host_and_scheme from django.utils.module_loading import import_string from django.utils.translation import gettext as _ from django.views.decorators.cache import never_cache @@ -49,20 +50,12 @@ get_remember_device_cookie, validate_remember_device_cookie, ) -try: - from django.utils.http import url_has_allowed_host_and_scheme -except ImportError: # django<3.0 - from django.utils.http import ( - is_safe_url as url_has_allowed_host_and_scheme, - ) - try: from django.contrib.auth.views import RedirectURLMixin except ImportError: # django<4.1 from django.contrib.auth.views import ( SuccessURLAllowedHostsMixin as RedirectURLMixin, ) - logger = logging.getLogger(__name__) REMEMBER_COOKIE_PREFIX = getattr(settings, 'TWO_FACTOR_REMEMBER_COOKIE_PREFIX', 'remember-cookie_') From 830915a8f002cdf44dccb20faca60ce4a48d90c9 Mon Sep 17 00:00:00 2001 From: Jaap Roes Date: Tue, 26 Jul 2022 09:09:47 +0200 Subject: [PATCH 06/24] Fixed #512 -- django.utils.baseconv module is deprecated --- two_factor/views/utils.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/two_factor/views/utils.py b/two_factor/views/utils.py index 4202201fb..ff061b063 100644 --- a/two_factor/views/utils.py +++ b/two_factor/views/utils.py @@ -8,7 +8,6 @@ from django.contrib.auth import load_backend from django.core.exceptions import SuspiciousOperation from django.core.signing import BadSignature, SignatureExpired -from django.utils import baseconv from django.utils.decorators import method_decorator from django.utils.encoding import force_bytes from django.utils.translation import gettext as _ @@ -16,6 +15,14 @@ from formtools.wizard.storage.session import SessionStorage from formtools.wizard.views import SessionWizardView +try: + from django.core.signing import b62_decode, b62_encode +except ImportError: # Django < 4.0 + # Deprecated in Django 4.0, removed in Django 5.0 + from django.utils import baseconv + b62_decode = baseconv.base62.decode + b62_encode = baseconv.base62.encode + logger = logging.getLogger(__name__) @@ -247,7 +254,7 @@ def get_remember_device_cookie(user, otp_device_id): 2. A hashed value of otp_device_id and the timestamp. 3. A hashed value of user.pk, user.password, otp_device_id and the timestamp. """ - timestamp = baseconv.base62.encode(int(time.time())) + timestamp = b62_encode(int(time.time())) cookie_key = hash_remember_device_cookie_key(otp_device_id) cookie_value = hash_remember_device_cookie_value(otp_device_id, user, timestamp) @@ -273,7 +280,7 @@ def validate_remember_device_cookie(cookie, user, otp_device_id): if input_cookie_value != cookie_value: raise BadSignature('Signature does not match') - timestamp_int = baseconv.base62.decode(timestamp) + timestamp_int = b62_decode(timestamp) age = time.time() - timestamp_int if age > settings.TWO_FACTOR_REMEMBER_COOKIE_AGE: raise SignatureExpired( From 4ee500a22502300bb1d174164f792227e8995d41 Mon Sep 17 00:00:00 2001 From: Tim Gates Date: Sat, 6 Aug 2022 07:06:38 +1000 Subject: [PATCH 07/24] docs: Fix a few typos There are small typos in: - docs/configuration.rst - tests/test_views_login.py - two_factor/views/core.py Fixes: - Should read `contrib` rather than `conrib`. - Should read `remembered` rather than `remebered`. - Should read `expire` rather than `exired`. - Should read `deactivated` rather than `deactived`. Signed-off-by: Tim Gates --- docs/configuration.rst | 4 ++-- tests/test_views_login.py | 2 +- two_factor/views/core.py | 10 +++++----- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/configuration.rst b/docs/configuration.rst index 738a833c0..0b9a03255 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -167,7 +167,7 @@ During a successful login with a token, the user may choose to remember this bro If the same user logs in again on the same browser, a token will not be requested, as the browser serves as a second factor. -The option to remember a browser is deactived by default. Set `TWO_FACTOR_REMEMBER_COOKIE_AGE` to activate. +The option to remember a browser is deactivated by default. Set `TWO_FACTOR_REMEMBER_COOKIE_AGE` to activate. The browser will be remembered as long as: @@ -177,7 +177,7 @@ The browser will be remembered as long as: The browser is remembered by setting a signed 'remember cookie'. -In order to invalidate remebered browsers after password resets, +In order to invalidate remembered browsers after password resets, the package relies on the `password` field of the `User` model. Please consider this in case you do not use the `password` field e.g. [django-auth-ldap](https://github.com/django-auth-ldap/django-auth-ldap) diff --git a/tests/test_views_login.py b/tests/test_views_login.py index 5e7f214ed..86441bde8 100644 --- a/tests/test_views_login.py +++ b/tests/test_views_login.py @@ -661,7 +661,7 @@ def test_wrong_signature(self): self.set_invalid_remember_cookie() - # Login but exired remember cookie + # Login but expire remember cookie response = self._post({'auth-username': 'bouke@example.com', 'auth-password': 'secret', 'login_view-current_step': 'auth'}) diff --git a/two_factor/views/core.py b/two_factor/views/core.py index 339b1d886..a0a528567 100644 --- a/two_factor/views/core.py +++ b/two_factor/views/core.py @@ -183,13 +183,13 @@ def done(self, form_list, **kwargs): self.request.session['next'] = self.get_success_url() return redirect('two_factor:setup') - # Copied from django.conrib.auth.views.LoginView (Branch: stable/1.11.x) + # Copied from django.contrib.auth.views.LoginView (Branch: stable/1.11.x) # https://github.com/django/django/blob/58df8aa40fe88f753ba79e091a52f236246260b3/django/contrib/auth/views.py#L63 def get_success_url(self): url = self.get_redirect_url() return url or resolve_url(settings.LOGIN_REDIRECT_URL) - # Copied from django.conrib.auth.views.LoginView (Branch: stable/1.11.x) + # Copied from django.contrib.auth.views.LoginView (Branch: stable/1.11.x) # https://github.com/django/django/blob/58df8aa40fe88f753ba79e091a52f236246260b3/django/contrib/auth/views.py#L67 def get_redirect_url(self): """Return the user-originating redirect URL if it's safe.""" @@ -374,7 +374,7 @@ def delete_cookies_from_response(self, response): response.delete_cookie(cookie) return response - # Copied from django.conrib.auth.views.LoginView (Branch: stable/1.11.x) + # Copied from django.contrib.auth.views.LoginView (Branch: stable/1.11.x) # https://github.com/django/django/blob/58df8aa40fe88f753ba79e091a52f236246260b3/django/contrib/auth/views.py#L49 @method_decorator(sensitive_post_parameters()) @method_decorator(csrf_protect) @@ -416,13 +416,13 @@ class SetupView(RedirectURLMixin, IdempotentSessionWizardView): # Other forms are dynamically added in get_form_list() ) - # Copied from django.conrib.auth.views.LoginView (Branch: stable/1.11.x) + # Copied from django.contrib.auth.views.LoginView (Branch: stable/1.11.x) # https://github.com/django/django/blob/58df8aa40fe88f753ba79e091a52f236246260b3/django/contrib/auth/views.py#L63 def get_success_url(self): url = self.get_redirect_url() return url or reverse('two_factor:setup_complete') - # Copied from django.conrib.auth.views.LoginView (Branch: stable/1.11.x) + # Copied from django.contrib.auth.views.LoginView (Branch: stable/1.11.x) # https://github.com/django/django/blob/58df8aa40fe88f753ba79e091a52f236246260b3/django/contrib/auth/views.py#L67 def get_redirect_url(self): """Return the user-originating redirect URL if it's safe.""" From c44a4227d70b0997e5ed22d7ec50b395e511607b Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 9 Aug 2022 20:42:48 +0100 Subject: [PATCH 08/24] [pre-commit.ci] pre-commit autoupdate (#523) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit updates: - [github.com/PyCQA/flake8: 4.0.1 → 5.0.4](https://github.com/PyCQA/flake8/compare/4.0.1...5.0.4) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 58b745e20..4cc658b28 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -6,7 +6,7 @@ repos: args: ['--check-only', '--diff'] - repo: https://github.com/PyCQA/flake8 - rev: 4.0.1 + rev: 5.0.4 hooks: - id: flake8 files: ^(example|tests|two_factor)/ From 5d1ce683ca77acccdef95ee09657e04a70bff6e2 Mon Sep 17 00:00:00 2001 From: Javier Paniagua Date: Fri, 19 Aug 2022 20:19:41 +0200 Subject: [PATCH 09/24] moved action text inside each method --- tests/test_email.py | 2 +- tests/test_utils.py | 7 ++++ two_factor/plugins/email/method.py | 11 ++++++ two_factor/plugins/email/utils.py | 21 ++++++++++++ two_factor/plugins/phonenumber/method.py | 17 ++++++++++ two_factor/plugins/phonenumber/models.py | 10 ------ two_factor/plugins/registry.py | 25 ++++++++++++++ two_factor/plugins/yubikey/method.py | 9 +++++ .../templates/two_factor/core/login.html | 16 +++------ .../templates/two_factor/profile/profile.html | 11 ++---- two_factor/templatetags/two_factor_tags.py | 17 ++++++++++ two_factor/views/core.py | 34 ++++++++++++++----- 12 files changed, 141 insertions(+), 39 deletions(-) create mode 100644 two_factor/plugins/email/utils.py create mode 100644 two_factor/templatetags/two_factor_tags.py diff --git a/tests/test_email.py b/tests/test_email.py index f57660f5c..93e2f33f8 100644 --- a/tests/test_email.py +++ b/tests/test_email.py @@ -140,7 +140,7 @@ def test_no_alternative(self): @mock.patch('two_factor.views.core.signals.user_verified.send') @override_settings(OTP_EMAIL_THROTTLE_FACTOR=0) def test_login(self, mock_signal): - device = self.user.emaildevice_set.create(name='default') + device = self.user.emaildevice_set.create(name='default', email='bouke@example.com') response = self.client.post(reverse('two_factor:login'), {'auth-username': 'bouke@example.com', diff --git a/tests/test_utils.py b/tests/test_utils.py index 8e1419e20..eb35d422a 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -6,6 +6,7 @@ from django_otp.util import random_hex from phonenumber_field.phonenumber import PhoneNumber +from two_factor.plugins.email.utils import mask_email from two_factor.plugins.phonenumber.models import PhoneDevice from two_factor.plugins.phonenumber.utils import ( backup_phones, format_phone_number, mask_phone_number, @@ -179,3 +180,9 @@ def test_format_phone_number(self): format_phone_number(PhoneNumber.from_string('+41524204242')), '+41 52 420 42 42' ) + + +class EmailUtilsTests(TestCase): + def test_mask_email(self): + self.assertEqual(mask_email('bouke@example.com'), 'b***e@example.com') + self.assertEqual(mask_email('tim@example.com'), 't**@example.com') diff --git a/two_factor/plugins/email/method.py b/two_factor/plugins/email/method.py index 3d290b8ff..13998146a 100644 --- a/two_factor/plugins/email/method.py +++ b/two_factor/plugins/email/method.py @@ -4,12 +4,16 @@ from two_factor.plugins.registry import MethodBase from .forms import AuthenticationTokenForm, DeviceValidationForm, EmailForm +from .utils import mask_email class EmailMethod(MethodBase): code = 'email' verbose_name = _('Email') + def get_devices(self, user): + return EmailDevice.objects.devices_for_user(user).all() + def recognize_device(self, device): return isinstance(device, EmailDevice) @@ -31,3 +35,10 @@ def get_device_from_setup_data(self, request, setup_data, **kwargs): def get_token_form_class(self): return AuthenticationTokenForm + + def get_action(self, device): + masked_email = mask_email(device.email) + return _('Send email to %s') % masked_email + + def get_verbose_action(self, device): + return _('We sent you an email, please enter the token we sent.') diff --git a/two_factor/plugins/email/utils.py b/two_factor/plugins/email/utils.py new file mode 100644 index 000000000..55081e3a5 --- /dev/null +++ b/two_factor/plugins/email/utils.py @@ -0,0 +1,21 @@ +def mask_email(email): + """ + Masks an email address, only first and last characters of the local part visible. + + Examples: + + * `j******e@example.com` + * `t**@example.com` + + :param email: str + :return: str + """ + local_part, domain = email.split('@') + local_part_length = len(local_part) + + if local_part_length < 4: + masked_local_part = local_part[0] + '*' * (local_part_length - 1) + else: + masked_local_part = local_part[0] + '*' * (local_part_length - 2) + local_part[-1] + + return f'{masked_local_part}@{domain}' diff --git a/two_factor/plugins/phonenumber/method.py b/two_factor/plugins/phonenumber/method.py index 28cad1551..8953e0f4c 100644 --- a/two_factor/plugins/phonenumber/method.py +++ b/two_factor/plugins/phonenumber/method.py @@ -4,9 +4,13 @@ from .forms import PhoneNumberForm from .models import PhoneDevice +from .utils import backup_phones, format_phone_number, mask_phone_number class PhoneMethodBase(MethodBase): + def get_devices(self, user): + return [device for device in backup_phones(user) if device.method == self.code] + def recognize_device(self, device): return isinstance(device, PhoneDevice) @@ -22,6 +26,19 @@ def get_device_from_setup_data(self, request, storage_data, **kwargs): number=storage_data.get(self.code, {}).get('number'), ) + def get_action(self, device): + number = mask_phone_number(format_phone_number(device.number)) + if device.method == 'sms': + return _('Send text message to %s') % number + else: + return _('Call number %s') % number + + def get_verbose_action(self, device): + if device.method == 'sms': + return _('We sent you a text message, please enter the token we sent.') + else: + return _('We are calling your phone right now, please enter the digits you hear.') + class PhoneCallMethod(PhoneMethodBase): code = 'call' diff --git a/two_factor/plugins/phonenumber/models.py b/two_factor/plugins/phonenumber/models.py index bca279ac5..0df802811 100644 --- a/two_factor/plugins/phonenumber/models.py +++ b/two_factor/plugins/phonenumber/models.py @@ -10,8 +10,6 @@ from two_factor.gateways import make_call, send_sms -from .utils import format_phone_number, mask_phone_number - PHONE_METHODS = ( ('call', _('Phone Call')), ('sms', _('Text Message')), @@ -76,13 +74,5 @@ def generate_challenge(self): else: send_sms(device=self, token=token) - @property - def generate_challenge_button_title(self): - number = mask_phone_number(format_phone_number(self.number)) - if self.method == 'sms': - return _('Send text message to %s') % number - else: - return _('Call number %s') % number - def get_throttle_factor(self): return getattr(settings, 'TWO_FACTOR_PHONE_THROTTLE_FACTOR', 1) diff --git a/two_factor/plugins/registry.py b/two_factor/plugins/registry.py index f23565356..985da6511 100644 --- a/two_factor/plugins/registry.py +++ b/two_factor/plugins/registry.py @@ -6,6 +6,16 @@ class MethodBase: verbose_name = None form_path = None + def get_devices(self, user): + raise NotImplementedError() + + def get_other_authentication_devices(self, user, main_device): + devices = self.get_devices(user) + return ( + device for device in devices + if (type(device) is not type(main_device)) or (device.pk != main_device.pk) + ) + def recognize_device(self, device): """ Return True if the device can be handled by this method. @@ -33,17 +43,32 @@ def get_token_form_class(self): return AuthenticationTokenForm + def get_action(self, device): + raise NotImplementedError() + + def get_verbose_action(self, device): + raise NotImplementedError() + class GeneratorMethod(MethodBase): code = 'generator' verbose_name = _('Token generator') form_path = 'two_factor.forms.TOTPDeviceForm' + def get_devices(self, user): + return user.totpdevice_set.all() + def get_setup_forms(self, *args): from two_factor.forms import TOTPDeviceForm return {'generator': TOTPDeviceForm} + def get_action(self, device): + return _('Enter the token generated by your token generator') + + def get_verbose_action(self, device): + return _('Please enter the token generated by your token generator.') + class MethodRegistry: _methods = [] diff --git a/two_factor/plugins/yubikey/method.py b/two_factor/plugins/yubikey/method.py index 2469b2002..461d6193e 100644 --- a/two_factor/plugins/yubikey/method.py +++ b/two_factor/plugins/yubikey/method.py @@ -10,6 +10,9 @@ class YubikeyMethod(MethodBase): code = 'yubikey' verbose_name = _('YubiKey') + def get_devices(self, user): + return RemoteYubikeyDevice.objects.filter(user=user) + def recognize_device(self, device): return isinstance(device, RemoteYubikeyDevice) @@ -30,3 +33,9 @@ def get_device_from_setup_data(self, request, setup_data, **kwargs): def get_token_form_class(self): return YubiKeyAuthenticationForm + + def get_action(self, device): + return _('Use your Yubikey device') + + def get_verbose_action(self, device): + return _('Please use your Yubikey device.') diff --git a/two_factor/templates/two_factor/core/login.html b/two_factor/templates/two_factor/core/login.html index 7aff4e395..e0ec08de9 100644 --- a/two_factor/templates/two_factor/core/login.html +++ b/two_factor/templates/two_factor/core/login.html @@ -1,5 +1,6 @@ {% extends "two_factor/_base_focus.html" %} {% load i18n %} +{% load two_factor_tags %} {% block content %}

{% block title %}{% trans "Login" %}{% endblock %}

@@ -7,16 +8,7 @@

{% block title %}{% trans "Login" %}{% endblock %}

{% if wizard.steps.current == 'auth' %}

{% blocktrans %}Enter your credentials.{% endblocktrans %}

{% elif wizard.steps.current == 'token' %} - {% if device.method == 'call' %} -

{% blocktrans trimmed %}We are calling your phone right now, please enter the - digits you hear.{% endblocktrans %}

- {% elif device.method == 'sms' %} -

{% blocktrans trimmed %}We sent you a text message, please enter the tokens we - sent.{% endblocktrans %}

- {% else %} -

{% blocktrans trimmed %}Please enter the tokens generated by your token - generator.{% endblocktrans %}

- {% endif %} +

{{ device|as_verbose_action }}

{% elif wizard.steps.current == 'backup' %}

{% blocktrans trimmed %}Use this form for entering backup tokens for logging in. These tokens have been generated for you to print and keep safe. Please @@ -30,12 +22,12 @@

{% block title %}{% trans "Login" %}{% endblock %}

{% if other_devices %} -

{% trans "Or, alternatively, use one of your backup phones:" %}

+

{% trans "Or, alternatively, use one of your other authentication methods:" %}

{% for other in other_devices %} {% endfor %}

{% endif %} diff --git a/two_factor/templates/two_factor/profile/profile.html b/two_factor/templates/two_factor/profile/profile.html index 9623766ea..0c86ba386 100644 --- a/two_factor/templates/two_factor/profile/profile.html +++ b/two_factor/templates/two_factor/profile/profile.html @@ -1,17 +1,12 @@ {% extends "two_factor/_base.html" %} {% load i18n %} +{% load two_factor_tags %} {% block content %}

{% block title %}{% trans "Account Security" %}{% endblock %}

{% if default_device %} - {% if default_device_type == 'TOTPDevice' %} -

{% trans "Tokens will be generated by your token generator." %}

- {% elif default_device_type == 'PhoneDevice' %} -

{% blocktrans with primary=default_device.generate_challenge_button_title %}Primary method: {{ primary }}{% endblocktrans %}

- {% elif default_device_type == 'RemoteYubikeyDevice' %} -

{% blocktrans %}Tokens will be generated by your YubiKey.{% endblocktrans %}

- {% endif %} +

{% blocktrans with primary=default_device|as_action %}Primary method: {{ primary }}{% endblocktrans %}

{% if available_phone_methods %}

{% trans "Backup Phone Numbers" %}

@@ -20,7 +15,7 @@

{% trans "Backup Phone Numbers" %}

    {% for phone in backup_phones %}
  • - {{ phone.generate_challenge_button_title }} + {{ phone|as_action }}
    {% csrf_token %} diff --git a/two_factor/templatetags/two_factor_tags.py b/two_factor/templatetags/two_factor_tags.py new file mode 100644 index 000000000..30b91e48e --- /dev/null +++ b/two_factor/templatetags/two_factor_tags.py @@ -0,0 +1,17 @@ +from django import template + +from two_factor.plugins.registry import registry + +register = template.Library() + + +@register.filter +def as_action(device): + method = registry.method_from_device(device) + return method.get_action(device) + + +@register.filter +def as_verbose_action(device): + method = registry.method_from_device(device) + return method.get_verbose_action(device) diff --git a/two_factor/views/core.py b/two_factor/views/core.py index a0a528567..6e3a7dba9 100644 --- a/two_factor/views/core.py +++ b/two_factor/views/core.py @@ -34,9 +34,7 @@ from django_otp.util import random_hex from two_factor import signals -from two_factor.plugins.phonenumber.utils import ( - backup_phones, get_available_phone_methods, -) +from two_factor.plugins.phonenumber.utils import get_available_phone_methods from two_factor.plugins.registry import registry from two_factor.utils import totp_digits @@ -278,19 +276,39 @@ def get_device(self, step=None): if not self.device_cache: challenge_device_id = self.request.POST.get('challenge_device', None) if challenge_device_id: - for device in backup_phones(self.get_user()): + for device in self.get_devices(): if device.persistent_id == challenge_device_id: self.device_cache = device break + if step == 'backup': try: self.device_cache = self.get_user().staticdevice_set.get(name='backup') except StaticDevice.DoesNotExist: pass + if not self.device_cache: self.device_cache = default_device(self.get_user()) + return self.device_cache + def get_devices(self): + user = self.get_user() + + devices = [] + for method in registry.get_methods(): + devices += list(method.get_devices(user)) + return devices + + def get_other_devices(self, main_device): + user = self.get_user() + + other_devices = [] + for method in registry.get_methods(): + other_devices += list(method.get_other_authentication_devices(user, main_device)) + + return other_devices + def render(self, form=None, **kwargs): """ If the user selected a device, ask the device to generate a challenge; @@ -317,10 +335,10 @@ def get_context_data(self, form, **kwargs): """ context = super().get_context_data(form, **kwargs) if self.steps.current == 'token': - context['device'] = self.get_device() - context['other_devices'] = [ - phone for phone in backup_phones(self.get_user()) - if phone != self.get_device()] + device = self.get_device() + context['device'] = device + context['other_devices'] = self.get_other_devices(device) + try: context['backup_tokens'] = self.get_user().staticdevice_set\ .get(name='backup').token_set.count() From 880d7b370c8b285c92abcf1262fefcc12f9bb5e5 Mon Sep 17 00:00:00 2001 From: Michal Dabski Date: Mon, 22 Aug 2022 20:57:46 +0100 Subject: [PATCH 10/24] Move Phone model to the correct app while keeping data --- .../migrations/0008_delete_phonedevice.py | 18 +++++++ .../phonenumber/migrations/0001_initial.py | 53 +++++++++++++++++++ .../phonenumber/migrations/__init__.py | 0 two_factor/plugins/phonenumber/models.py | 2 +- 4 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 two_factor/migrations/0008_delete_phonedevice.py create mode 100644 two_factor/plugins/phonenumber/migrations/0001_initial.py create mode 100644 two_factor/plugins/phonenumber/migrations/__init__.py diff --git a/two_factor/migrations/0008_delete_phonedevice.py b/two_factor/migrations/0008_delete_phonedevice.py new file mode 100644 index 000000000..922c3fdcc --- /dev/null +++ b/two_factor/migrations/0008_delete_phonedevice.py @@ -0,0 +1,18 @@ +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('two_factor', '0007_auto_20201201_1019'), + ] + + operations = [ + migrations.SeparateDatabaseAndState( + state_operations=[ + migrations.DeleteModel( + name='PhoneDevice', + ), + ], + ), + ] diff --git a/two_factor/plugins/phonenumber/migrations/0001_initial.py b/two_factor/plugins/phonenumber/migrations/0001_initial.py new file mode 100644 index 000000000..bed08d588 --- /dev/null +++ b/two_factor/plugins/phonenumber/migrations/0001_initial.py @@ -0,0 +1,53 @@ +import django.db.models.deletion +import django_otp.util +import phonenumber_field.modelfields +from django.conf import settings +from django.db import migrations, models + +import two_factor.plugins.phonenumber.models + + +class Migration(migrations.Migration): + initial = True + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('two_factor', '0008_delete_phonedevice'), + ] + + operations = [ + migrations.SeparateDatabaseAndState( + state_operations=[ + migrations.CreateModel( + name='PhoneDevice', + fields=[ + ('id', + models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('name', models.CharField(help_text='The human-readable name of this device.', max_length=64)), + ('confirmed', models.BooleanField(default=True, help_text='Is this device ready for use?')), + ('throttling_failure_timestamp', models.DateTimeField( + blank=True, default=None, + help_text='A timestamp of the last failed verification attempt. ' + 'Null if last attempt succeeded.', + null=True)), + ('throttling_failure_count', + models.PositiveIntegerField(default=0, help_text='Number of successive failed attempts.')), + ('number', phonenumber_field.modelfields.PhoneNumberField(max_length=128, region=None)), + ('key', models.CharField(default=django_otp.util.random_hex, + help_text='Hex-encoded secret key', + max_length=40, + validators=[two_factor.plugins.phonenumber.models.key_validator])), + ('method', + models.CharField(choices=[('call', 'Phone Call'), ('sms', 'Text Message')], max_length=4, + verbose_name='method')), + ('user', models.ForeignKey(help_text='The user that this device belongs to.', + on_delete=django.db.models.deletion.CASCADE, + to=settings.AUTH_USER_MODEL)), + ], + options={ + 'db_table': 'two_factor_phonedevice', + }, + ), + ], + ), + ] diff --git a/two_factor/plugins/phonenumber/migrations/__init__.py b/two_factor/plugins/phonenumber/migrations/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/two_factor/plugins/phonenumber/models.py b/two_factor/plugins/phonenumber/models.py index 0df802811..f087b91be 100644 --- a/two_factor/plugins/phonenumber/models.py +++ b/two_factor/plugins/phonenumber/models.py @@ -26,7 +26,7 @@ class PhoneDevice(ThrottlingMixin, Device): Model with phone number and token seed linked to a user. """ class Meta: - app_label = 'two_factor' + db_table = 'two_factor_phonedevice' number = PhoneNumberField() key = models.CharField(max_length=40, From fe6c940d93a254dd7d2623c3a4752f69ea6ac1db Mon Sep 17 00:00:00 2001 From: Ivan Tham Date: Tue, 23 Aug 2022 04:07:45 +0800 Subject: [PATCH 11/24] Add support for Django 4.1 --- CHANGELOG.md | 1 + README.rst | 2 +- setup.py | 1 + tests/settings.py | 2 ++ tox.ini | 4 +++- 5 files changed, 8 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c6d1d6a3f..72da40b5a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Added - Enforcing a redirect to setup of otp device when none available for user [#550](https://github.com/jazzband/django-two-factor-auth/pull/500) + - Confirmed Django 4.1 support ### Removed - Django 2.2, 3.0, and 3.1 support diff --git a/README.rst b/README.rst index e7ba29a43..d64bee815 100644 --- a/README.rst +++ b/README.rst @@ -38,7 +38,7 @@ is optional, it improves account security control over ``django.contrib.sessions``. Compatible with supported Django and Python versions. At the moment of writing that -includes 3.2 and 4.0 on Python 3.7, 3.8, 3.9 and 3.10. +includes 3.2, 4.0 and 4.1 on Python 3.7, 3.8, 3.9 and 3.10. Documentation is available at `readthedocs.org`_. diff --git a/setup.py b/setup.py index 8985c3ac3..8c8192f16 100644 --- a/setup.py +++ b/setup.py @@ -32,6 +32,7 @@ 'Framework :: Django', 'Framework :: Django :: 3.2', 'Framework :: Django :: 4.0', + 'Framework :: Django :: 4.1', 'Intended Audience :: Developers', 'License :: OSI Approved :: MIT License', 'Operating System :: OS Independent', diff --git a/tests/settings.py b/tests/settings.py index 902d98e09..b9c56643a 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -77,6 +77,8 @@ }, ] +DEFAULT_AUTO_FIELD = 'django.db.models.AutoField' + TWO_FACTOR_PATCH_ADMIN = False AUTH_USER_MODEL = os.environ.get('AUTH_USER_MODEL', 'auth.User') diff --git a/tox.ini b/tox.ini index 1bacdb87c..6decc5d67 100644 --- a/tox.ini +++ b/tox.ini @@ -3,7 +3,7 @@ minversion = 1.8 envlist = py{37,38,39,310}-dj32-{normal,yubikey,custom_user}, - py{38,39,310}-dj40-{normal,yubikey,custom_user} + py{38,39,310}-dj{40,41}-{normal,yubikey,custom_user} py{38,39,310}-djmain-{normal,yubikey,custom_user} [gh-actions] @@ -17,6 +17,7 @@ python = DJANGO = 3.2: dj32 4.0: dj40 + 4.1: dj41 main: djmain VARIANT = normal: normal @@ -36,6 +37,7 @@ basepython = deps = dj32: Django<4.0 dj40: Django<4.1 + dj41: Django<4.2 djmain: https://github.com/django/django/archive/main.tar.gz yubikey: django-otp-yubikey coverage From f427f59c03ba091cd1025ad304535e697492b0eb Mon Sep 17 00:00:00 2001 From: EngFarisAlsmawi Date: Sat, 27 Aug 2022 19:22:16 +0300 Subject: [PATCH 12/24] Fixed #530 - Avoid crash for email devices without email --- tests/test_email.py | 13 +++++++++++++ two_factor/plugins/email/method.py | 4 ++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/tests/test_email.py b/tests/test_email.py index 93e2f33f8..049c75ab9 100644 --- a/tests/test_email.py +++ b/tests/test_email.py @@ -176,3 +176,16 @@ def test_login(self, mock_signal): # Check that the signal was fired. mock_signal.assert_called_with(sender=mock.ANY, request=mock.ANY, user=self.user, device=device) + + def test_device_without_email(self): + self.user.emaildevice_set.create(name="default") + response = self.client.get(reverse("two_factor:profile")) + self.assertNotContains( + response, + "AttributeError: 'NoneType' object has no attribute 'split'", + ) + + def test_device_user_without_email(self): + self.user.email = "" + self.user.save() + self.test_device_without_email() diff --git a/two_factor/plugins/email/method.py b/two_factor/plugins/email/method.py index 13998146a..94afa5da6 100644 --- a/two_factor/plugins/email/method.py +++ b/two_factor/plugins/email/method.py @@ -37,8 +37,8 @@ def get_token_form_class(self): return AuthenticationTokenForm def get_action(self, device): - masked_email = mask_email(device.email) - return _('Send email to %s') % masked_email + email = device.email or device.user.email + return _('Send email to %s') % (email and mask_email(email) or None,) def get_verbose_action(self, device): return _('We sent you an email, please enter the token we sent.') From 0feb3cc62e4c38a3dd8851a021ee88c8944c7282 Mon Sep 17 00:00:00 2001 From: Claude Paroz Date: Thu, 15 Sep 2022 16:13:41 +0200 Subject: [PATCH 13/24] Write INSTALLED_APPS as a list instead of tuple Co-authored-by: Javier Paniagua --- docs/installation.rst | 8 ++++---- example/settings.py | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/installation.rst b/docs/installation.rst index 5ae666176..3daa668ce 100644 --- a/docs/installation.rst +++ b/docs/installation.rst @@ -28,7 +28,7 @@ Add the following apps to the ``INSTALLED_APPS``: .. code-block:: python - INSTALLED_APPS = ( + INSTALLED_APPS = [ ... 'django_otp', 'django_otp.plugins.otp_static', @@ -38,7 +38,7 @@ Add the following apps to the ``INSTALLED_APPS``: 'two_factor.plugins.phonenumber', # <- if you want phone number capability. 'two_factor.plugins.email', # <- if you want email capability. 'two_factor.plugins.yubikey', # <- for yubikey capability. - ) + ] Add the ``django-otp`` middleware to your ``MIDDLEWARE``. Make sure it comes after ``AuthenticationMiddleware``: @@ -89,10 +89,10 @@ Add the following app to the ``INSTALLED_APPS``: .. code-block:: python - INSTALLED_APPS = ( + INSTALLED_APPS = [ ... 'otp_yubikey', - ) + ] This plugin also requires adding a validation service, through which YubiKeys will be verified. Normally, you'd use the YubiCloud for this. In the Django diff --git a/example/settings.py b/example/settings.py index 13b107c2d..ef6f44ca6 100644 --- a/example/settings.py +++ b/example/settings.py @@ -52,7 +52,7 @@ }, ] -INSTALLED_APPS = ( +INSTALLED_APPS = [ 'django.contrib.auth', 'django.contrib.contenttypes', 'user_sessions', @@ -68,7 +68,7 @@ 'debug_toolbar', 'bootstrapform' -) +] LOGOUT_REDIRECT_URL = 'home' From 228f6c7b9a261da53069cfbc1e75ee55bac434a3 Mon Sep 17 00:00:00 2001 From: jpaniagualaconich Date: Thu, 15 Sep 2022 18:37:59 +0200 Subject: [PATCH 14/24] removed get_available_methods It's been superseded by MethodRegistry.get_methods --- two_factor/utils.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/two_factor/utils.py b/two_factor/utils.py index 8f383436a..3f69c24d2 100644 --- a/two_factor/utils.py +++ b/two_factor/utils.py @@ -3,8 +3,6 @@ from django.conf import settings from django_otp import devices_for_user -from two_factor.plugins.registry import registry - USER_DEFAULT_DEVICE_ATTR_NAME = "_default_device" @@ -57,7 +55,3 @@ def totp_digits(): for totp tokens. Defaults to 6 """ return getattr(settings, 'TWO_FACTOR_TOTP_DIGITS', 6) - - -def get_available_methods(): - return [(m.code, m.verbose_name) for m in registry.get_methods()] From 1878a4d6b8017d1a907b2b46f6991ba726a019a3 Mon Sep 17 00:00:00 2001 From: Javier Paniagua Date: Thu, 15 Sep 2022 17:40:56 +0200 Subject: [PATCH 15/24] avoid hard reference to generator --- two_factor/forms.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/two_factor/forms.py b/two_factor/forms.py index 50551fabe..74421c679 100644 --- a/two_factor/forms.py +++ b/two_factor/forms.py @@ -14,14 +14,16 @@ class MethodForm(forms.Form): method = forms.ChoiceField(label=_("Method"), - initial='generator', widget=forms.RadioSelect) def __init__(self, **kwargs): super().__init__(**kwargs) - self.fields['method'].choices = [ + + method = self.fields['method'] + method.choices = [ (m.code, m.verbose_name) for m in registry.get_methods() ] + method.initial = method.choices[0][0] class DeviceValidationForm(forms.Form): From 4cbd8fd6d4b202fb8e5ab372b6ad31b69b391c80 Mon Sep 17 00:00:00 2001 From: Javier Paniagua Date: Thu, 15 Sep 2022 17:41:34 +0200 Subject: [PATCH 16/24] typo --- two_factor/forms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/two_factor/forms.py b/two_factor/forms.py index 74421c679..9f13cc5c0 100644 --- a/two_factor/forms.py +++ b/two_factor/forms.py @@ -136,7 +136,7 @@ def __init__(self, user, initial_device, **kwargs): self.user = user self.initial_device = initial_device - # Add a field to remeber this browser. + # Add a field to remember this browser. if getattr(settings, 'TWO_FACTOR_REMEMBER_COOKIE_AGE', None): if settings.TWO_FACTOR_REMEMBER_COOKIE_AGE < 3600: minutes = int(settings.TWO_FACTOR_REMEMBER_COOKIE_AGE / 60) From 21dfc437533f7a5d6defd97e5351dbc165eef56a Mon Sep 17 00:00:00 2001 From: Javier Paniagua Date: Thu, 15 Sep 2022 17:42:22 +0200 Subject: [PATCH 17/24] used kwargs to avoid confusion --- two_factor/forms.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/two_factor/forms.py b/two_factor/forms.py index 9f13cc5c0..c2ce31655 100644 --- a/two_factor/forms.py +++ b/two_factor/forms.py @@ -36,8 +36,8 @@ class DeviceValidationForm(forms.Form): 'invalid_token': _('Entered token is not valid.'), } - def __init__(self, device, **args): - super().__init__(**args) + def __init__(self, device, **kwargs): + super().__init__(**kwargs) self.device = device def clean_token(self): From 445a023768974a6ff74737e539bbc9c076294675 Mon Sep 17 00:00:00 2001 From: Javier Paniagua Date: Thu, 15 Sep 2022 17:58:28 +0200 Subject: [PATCH 18/24] inspect form params to build form kwargs --- two_factor/views/core.py | 52 +++++++++++++++++++++++----------------- 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/two_factor/views/core.py b/two_factor/views/core.py index 6e3a7dba9..c0e4123da 100644 --- a/two_factor/views/core.py +++ b/two_factor/views/core.py @@ -3,6 +3,7 @@ import warnings from base64 import b32encode from binascii import unhexlify +from inspect import signature from uuid import uuid4 import django_otp @@ -203,19 +204,20 @@ def get_redirect_url(self): return redirect_to if url_is_safe else '' def get_form_kwargs(self, step=None): - """ - AuthenticationTokenForm requires the user kwarg. - """ - if step == 'auth': - return { - 'request': self.request - } - if step in ('token', 'backup'): - return { - 'user': self.get_user(), - 'initial_device': self.get_device(step), - } - return {} + if step is None: + return {} + + form_class = self.get_form_list()[step] + form_params = signature(form_class).parameters + + kwargs = {} + if 'user' in form_params: + kwargs['user'] = self.get_user() + if 'initial_device' in form_params: + kwargs['initial_device'] = self.get_device(step) + if 'request' in form_params: + kwargs['request'] = self.request + return kwargs def get_done_form_list(self): """ @@ -536,16 +538,22 @@ def done(self, form_list, **kwargs): return redirect(self.get_success_url()) def get_form_kwargs(self, step=None): + if step is None: + return {} + + form_class = self.get_form_list()[step] + form_params = signature(form_class).parameters + kwargs = {} - if step == 'generator': - kwargs.update({ - 'key': self.get_key(step), - 'user': self.request.user, - }) - if step in ('validation', 'yubikey'): - kwargs.update({ - 'device': self.get_device() - }) + if 'key' in form_params: + kwargs['key'] = self.get_key(step) + if 'user' in form_params: + kwargs['user'] = self.request.user + if 'device' in form_params: + kwargs['device'] = self.get_device() + if 'request' in form_params: + kwargs['request'] = self.request + metadata = self.get_form_metadata(step) if metadata: kwargs.update({ From 6307db660bed4539c433069135940c9d35a1baff Mon Sep 17 00:00:00 2001 From: yopiti Date: Wed, 11 Aug 2021 10:47:03 +0200 Subject: [PATCH 19/24] Replace d-none class by hidden attribute Using hidden attribute, to avoid conflict with pages that are not using Bootstrap as CSS framework --- two_factor/templates/two_factor/core/login.html | 2 +- two_factor/templates/two_factor/core/phone_register.html | 2 +- two_factor/templates/two_factor/core/setup.html | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/two_factor/templates/two_factor/core/login.html b/two_factor/templates/two_factor/core/login.html index e0ec08de9..a52ada14b 100644 --- a/two_factor/templates/two_factor/core/login.html +++ b/two_factor/templates/two_factor/core/login.html @@ -19,7 +19,7 @@

    {% block title %}{% trans "Login" %}{% endblock %}

    {% include "two_factor/_wizard_forms.html" %} {# hidden submit button to enable [enter] key #} - + {% if other_devices %}

    {% trans "Or, alternatively, use one of your other authentication methods:" %}

    diff --git a/two_factor/templates/two_factor/core/phone_register.html b/two_factor/templates/two_factor/core/phone_register.html index 034ce60ff..3bbecc1a1 100644 --- a/two_factor/templates/two_factor/core/phone_register.html +++ b/two_factor/templates/two_factor/core/phone_register.html @@ -17,7 +17,7 @@

    {% block title %}{% trans "Add Backup Phone" %}{% endblock %}

    {% include "two_factor/_wizard_forms.html" %} {# hidden submit button to enable [enter] key #} - + {% include "two_factor/_wizard_actions.html" %} diff --git a/two_factor/templates/two_factor/core/setup.html b/two_factor/templates/two_factor/core/setup.html index ac2bb0e36..1064061cb 100644 --- a/two_factor/templates/two_factor/core/setup.html +++ b/two_factor/templates/two_factor/core/setup.html @@ -49,7 +49,7 @@

    {% block title %}{% trans "Enable Two-Factor Authentication" %}{% endblock % {% include "two_factor/_wizard_forms.html" %} {# hidden submit button to enable [enter] key #} - + {% include "two_factor/_wizard_actions.html" %} From 8d1ecfe64fdfcc2e304dbd5134db7b88456025f7 Mon Sep 17 00:00:00 2001 From: Serg Tereshchenko Date: Sat, 3 Sep 2022 22:32:50 +0300 Subject: [PATCH 20/24] refactor: Move registry.get_methods() call to separate method, to allow customization Refs #533 --- two_factor/views/core.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/two_factor/views/core.py b/two_factor/views/core.py index c0e4123da..1defd0fe5 100644 --- a/two_factor/views/core.py +++ b/two_factor/views/core.py @@ -482,7 +482,7 @@ def get_form_list(self): """ form_list = super().get_form_list() - available_methods = registry.get_methods() + available_methods = self.get_available_methods() if len(available_methods) == 1: form_list.pop('method', None) method_key = available_methods[0].code @@ -497,6 +497,9 @@ def get_form_list(self): form_list['validation'] = DeviceValidationForm return form_list + def get_available_methods(self): + return registry.get_methods() + def render_next_step(self, form, **kwargs): """ In the validation step, ask the device to generate a challenge. From 96bbd1afd54421016f6db9b88d44f3128e406198 Mon Sep 17 00:00:00 2001 From: Javier Paniagua Date: Tue, 20 Sep 2022 09:02:23 +0200 Subject: [PATCH 21/24] Implement the WebAuthn method --- CHANGELOG.md | 3 +- MANIFEST.in | 1 + Makefile | 8 +- docs/configuration.rst | 105 +++++++++++++ docs/installation.rst | 38 +++++ example/settings.py | 2 + example/settings_webauthn.py | 3 + requirements_dev.txt | 4 + requirements_e2e.txt | 3 + setup.py | 1 + tests/settings.py | 12 ++ tox.ini | 11 +- two_factor/plugins/webauthn/__init__.py | 0 two_factor/plugins/webauthn/admin.py | 9 ++ two_factor/plugins/webauthn/apps.py | 52 +++++++ two_factor/plugins/webauthn/forms.py | 144 ++++++++++++++++++ two_factor/plugins/webauthn/method.py | 65 ++++++++ .../webauthn/migrations/0001_initial.py | 58 +++++++ .../plugins/webauthn/migrations/__init__.py | 0 two_factor/plugins/webauthn/models.py | 19 +++ .../static/two_factor/js/webauthn_utils.js | 21 +++ .../two_factor_webauthn/create_credential.js | 37 +++++ .../two_factor_webauthn/get_credential.js | 37 +++++ two_factor/plugins/webauthn/tests/__init__.py | 0 two_factor/plugins/webauthn/tests/test_e2e.py | 114 ++++++++++++++ .../plugins/webauthn/tests/test_forms.py | 47 ++++++ .../plugins/webauthn/tests/test_utils.py | 61 ++++++++ .../webauthn/tests/test_views_setup.py | 60 ++++++++ two_factor/plugins/webauthn/urls.py | 18 +++ two_factor/plugins/webauthn/utils.py | 142 +++++++++++++++++ two_factor/plugins/webauthn/views.py | 43 ++++++ two_factor/templates/two_factor/_base.html | 1 + .../templates/two_factor/core/login.html | 4 + .../templates/two_factor/core/setup.html | 4 + two_factor/templatetags/__init__.py | 0 two_factor/urls.py | 18 ++- two_factor/views/core.py | 11 +- 37 files changed, 1145 insertions(+), 11 deletions(-) create mode 100644 example/settings_webauthn.py create mode 100644 requirements_e2e.txt create mode 100644 two_factor/plugins/webauthn/__init__.py create mode 100644 two_factor/plugins/webauthn/admin.py create mode 100644 two_factor/plugins/webauthn/apps.py create mode 100644 two_factor/plugins/webauthn/forms.py create mode 100644 two_factor/plugins/webauthn/method.py create mode 100644 two_factor/plugins/webauthn/migrations/0001_initial.py create mode 100644 two_factor/plugins/webauthn/migrations/__init__.py create mode 100644 two_factor/plugins/webauthn/models.py create mode 100644 two_factor/plugins/webauthn/static/two_factor/js/webauthn_utils.js create mode 100644 two_factor/plugins/webauthn/templates/two_factor_webauthn/create_credential.js create mode 100644 two_factor/plugins/webauthn/templates/two_factor_webauthn/get_credential.js create mode 100644 two_factor/plugins/webauthn/tests/__init__.py create mode 100644 two_factor/plugins/webauthn/tests/test_e2e.py create mode 100644 two_factor/plugins/webauthn/tests/test_forms.py create mode 100644 two_factor/plugins/webauthn/tests/test_utils.py create mode 100644 two_factor/plugins/webauthn/tests/test_views_setup.py create mode 100644 two_factor/plugins/webauthn/urls.py create mode 100644 two_factor/plugins/webauthn/utils.py create mode 100644 two_factor/plugins/webauthn/views.py create mode 100644 two_factor/templatetags/__init__.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 72da40b5a..bde04afb2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,8 @@ ### Added - Enforcing a redirect to setup of otp device when none available for user [#550](https://github.com/jazzband/django-two-factor-auth/pull/500) - - Confirmed Django 4.1 support +- Confirmed Django 4.1 support +- WebAuthn support ### Removed - Django 2.2, 3.0, and 3.1 support diff --git a/MANIFEST.in b/MANIFEST.in index 768f8364a..b4917a920 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -2,3 +2,4 @@ include *.rst LICENSE prune two_factor/locale recursive-include two_factor/locale * recursive-include two_factor/templates * +recursive-include two_factor/plugins/*/static * diff --git a/Makefile b/Makefile index 489e85ffd..613e7e263 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,3 @@ -TARGET?=tests - .PHONY: docs flake8 example test coverage migrations docs: @@ -13,6 +11,12 @@ example: DJANGO_SETTINGS_MODULE=example.settings PYTHONPATH=. \ django-admin runserver +example-webauthn: + DJANGO_SETTINGS_MODULE=example.settings_webauthn PYTHONPATH=. \ + django-admin migrate + DJANGO_SETTINGS_MODULE=example.settings_webauthn PYTHONPATH=. \ + django-admin runserver + test: DJANGO_SETTINGS_MODULE=tests.settings PYTHONPATH=. \ django-admin test ${TARGET} diff --git a/docs/configuration.rst b/docs/configuration.rst index 0b9a03255..2c5d8d177 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -160,6 +160,111 @@ Fake Gateway .. _python-qrcode: https://pypi.python.org/pypi/qrcode .. _`the upstream ticket`: https://code.google.com/p/google-authenticator/issues/detail?id=327 +WebAuthn Settings +----------------- + +Start by providing a value for the following setting: + +``TWO_FACTOR_WEBAUTHN_RP_NAME`` (default: ``None``) + The human-palatable identifier for the `Relying Party`_. You **MUST** name your application. Failing to do so will + raise an ``ImproperlyConfigured`` exception. + +The defaults provided for all other settings should be enough to enable the use of fingerprint readers, security keys +and android phones (Chrome-based browsers only). + +Tweak the following settings if you want to restrict the types of devices that can be used and the information that +will be sent to your application after the authentication takes place: + +``TWO_FACTOR_WEBAUTHN_AUTHENTICATOR_ATTACHMENT`` (default: ``None``) + The preferred `Authenticator Attachment`_ modality. + Possible values: ``'platform'`` (like an embedded fingerprint reader), ``'cross-platform'`` (such as a usb security + key). The default is to accept all attachment modalities. + +``TWO_FACTOR_WEBAUTHN_PREFERRED_TRANSPORTS`` (default: ``None``) + A list of preferred communication transports that will be set for all registered authenticators. **This can be + used to optimize user interaction at authentication time. Its implementation is highly browser-dependent and may + even be disregarded.** + + Chrome uses this to filter out credentials that do not use any of the transports listed. + For example, if set to ``['usb', 'internal']`` Chrome will not attempt to authenticate the user with authenticators + that communicate using CaBLE (e.g., android phones). + + Possible values for each element in the list are members of ``webauthn.helpers.structs.AuthenticatorTransport``. The + default is to accept all transports. + +``TWO_FACTOR_WEBAUTHN_UV_REQUIREMENT`` (default: ``'discouraged'``) + The type of `User Verification`_ that is required. Verification ranges from a simple test of user presence such as + by touching a button to more thorough checks like using biometrics or requiring user PIN input. + Possible values: ``'discouraged'``, ``'preferred'``, ``'required'``. + +``TWO_FACTOR_WEBAUTHN_ATTESTATION_CONVEYANCE`` (default: ``'none'``) + The type of `Attestation Conveyance`_. A `Relying Party`_ may want to verify attestations to ensure that + only authentication devices from certain approved vendors can be used. Depending on the level of conveyance, the + attestation could include potentially identifying information, resulting in an additional prompt to the users so + they can decide if they want to proceed. + Possible values: ``'none'``, ``'indirect'`` and ``'direct'``. The ``'enterprise'`` conveyance type is not supported + and will result in ``ImproperlyConfigured`` being raised. + + .. warning:: + Setting conveyance to other than ``'none'`` enables attestation verification against a list of root certificates. + If the list of root certificates for a particular attestation statement format is empty, **then verification will + always pass**. + + ``'fido-u2f'``, ``'packed'`` and ``'tpm'`` do not come pre-configured with root certificates. Download the + additional certificates that you needed for your particular device and use the + ``TWO_FACTOR_WEBAUTHN_PEM_ROOT_CERTS_BYTES_BY_FMT`` setting below. + +``TWO_FACTOR_WEBAUTHN_PEM_ROOT_CERTS_BYTES_BY_FMT`` (default: ``None``) + A mapping of attestation statement formats to lists of Root Certificates, provided as bytes. These will be used in + addition to those already provided by ``py_webauthn`` to verify attestation objects. + + **Example:** + + If you want to verify attestations made by a Yubikey, get `Yubico's root CA`_ and use it as follows: + + .. code-block:: python + + yubico_u2f_ca = """ + -----BEGIN CERTIFICATE----- + (Yubico's root CA goes here) + -----END CERTIFICATE----- + """ + + root_ca_list = [yubico_u2f_ca.encode('ascii')] + + TWO_FACTOR_WEBAUTHN_PEM_ROOT_CERTS_BYTES_BY_FMT = { + AttestationFormat.PACKED: root_ca_list, + AttestationFormat.FIDO_U2F: root_ca_list, + } + +The following settings control how the attributes for WebAuthn entities are built: + +``TWO_FACTOR_WEBAUTHN_ENTITIES_FORM_MIXIN`` (default: ``'two_factor.webauthn.utils.WebauthnEntitiesFormMixin'``) + A mixin to provide WebAuthn entities (user and `Relying Party`_) needed during setup and authentication. Although + the default works in most cases you can provide your own methods to build the different attributes that + are required by these entities (e.g., if you use a custom `User` model). + +``TWO_FACTOR_WEBAUTHN_RP_ID`` (default: ``None``) + The default form mixin uses this setting to specify the domain of the `Relying Party`_. By default, the relying + party ID is ``None`` i.e. the current domain returned by `HttpRequest.get_host()`_ will be used. You may want to set + it to a higher-level domain if your application has several sub-domains (e.g., ``www.example.com`` for web and + ``m.example.com`` for the mobile version, meaning you may want to set this value to ``'example.com'`` so credentials + are valid for both versions). + +WebAuthn devices support throttling too: + +``TWO_FACTOR_WEBAUTHN_THROTTLE_FACTOR`` (default: ``1``) + This controls the rate of throttling. The sequence of 1, 2, 4, 8... seconds is + multiplied by this factor to define the delay imposed after 1, 2, 3, 4... + successive failures. Set to ``0`` to disable throttling completely. + +.. _`Relying Party`: https://w3c.github.io/webauthn/#webauthn-relying-party +.. _`Authenticator Attachment`: https://www.w3.org/TR/webauthn/#enum-attachment +.. _`User Verification`: https://www.w3.org/TR/webauthn-2/#enum-userVerificationRequirement +.. _`Attestation Conveyance`: https://www.w3.org/TR/webauthn-2/#enum-attestation-convey +.. _`Yubico's Root CA`: https://developers.yubico.com/U2F/Attestation_and_Metadata/ +.. _`HttpRequest.get_host()`: https://docs.djangoproject.com/en/4.0/ref/request-response/#django.http.HttpRequest.get_host + Remember Browser ---------------- diff --git a/docs/installation.rst b/docs/installation.rst index 3daa668ce..0fe990576 100644 --- a/docs/installation.rst +++ b/docs/installation.rst @@ -116,5 +116,43 @@ You could also do this using Django's `manage.py shell`: ... ) +.. _webauthn-setup: + +WebAuthn Setup +-------------- + +In order to support WebAuthn_ devices, you have to install the py_webauthn_ package. +It's a ``django-two-factor-auth`` extra so you can select it at install time: + +.. code-block:: console + + $ pip install django-two-factor-auth[webauthn] + +You need to include the plugin in your Django settings: + +.. code-block:: python + + INSTALLED_APPS = [ + ... + 'two_factor.plugins.webauthn', + ] + +WebAuthn also requires your service to be reachable using HTTPS. +An exception is made if the domain is ``localhost``, which can be served using plain HTTP. + +If you use a different domain, don't forget to set ``SECURE_PROXY_SSL_HEADER`` in your Django settings accordingly: + +.. code-block:: python + + SECURE_PROXY_SSL_HEADER = ('HTTP_X_FORWARDED_PROTO', 'https') + +You can try a WebAuthn-enabled version of the example app that is reachable at http://localhost:8000: + +.. code-block:: console + + $ make example-webauthn + .. _PyPI: https://pypi.python.org/pypi/django-two-factor-auth .. _Yubikeys: https://www.yubico.com/products/yubikey-hardware/ +.. _WebAuthn: https://www.w3.org/TR/webauthn/ +.. _py_webauthn: https://pypi.org/project/webauthn/ diff --git a/example/settings.py b/example/settings.py index ef6f44ca6..775a4ac81 100644 --- a/example/settings.py +++ b/example/settings.py @@ -100,6 +100,8 @@ TWO_FACTOR_REMEMBER_COOKIE_AGE = 120 # Set to 2 minute for testing +TWO_FACTOR_WEBAUTHN_RP_NAME = 'Demo Application' + SESSION_ENGINE = 'user_sessions.backends.db' EMAIL_BACKEND = 'django.core.mail.backends.console.EmailBackend' diff --git a/example/settings_webauthn.py b/example/settings_webauthn.py new file mode 100644 index 000000000..5a66e1030 --- /dev/null +++ b/example/settings_webauthn.py @@ -0,0 +1,3 @@ +from .settings import * # noqa: F403 + +INSTALLED_APPS.extend(['two_factor.plugins.webauthn']) # noqa: F405 diff --git a/requirements_dev.txt b/requirements_dev.txt index a03692141..4e9def5c7 100644 --- a/requirements_dev.txt +++ b/requirements_dev.txt @@ -13,6 +13,10 @@ django-debug-toolbar django-bootstrap-form django-user-sessions +# Example app (WebAuthn) + +webauthn~=1.6.0 + # Testing coverage diff --git a/requirements_e2e.txt b/requirements_e2e.txt new file mode 100644 index 000000000..bd8c98491 --- /dev/null +++ b/requirements_e2e.txt @@ -0,0 +1,3 @@ +# test with selenium +selenium~=3.141.0 +webdriver-manager~=3.4.2 diff --git a/setup.py b/setup.py index 8c8192f16..6aa8bdc6a 100644 --- a/setup.py +++ b/setup.py @@ -21,6 +21,7 @@ extras_require={ 'call': ['twilio>=6.0'], 'sms': ['twilio>=6.0'], + 'webauthn': ['webauthn>=1.6.0,<1.99', 'pydantic>=1.9.0,<1.99'], 'yubikey': ['django-otp-yubikey'], 'phonenumbers': ['phonenumbers>=7.0.9,<8.99',], 'phonenumberslite': ['phonenumberslite>=7.0.9,<8.99',], diff --git a/tests/settings.py b/tests/settings.py index b9c56643a..17783b2c5 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -5,6 +5,11 @@ except ImportError: otp_yubikey = None +try: + import webauthn +except ImportError: + webauthn = None + BASE_DIR = os.path.dirname(__file__) SECRET_KEY = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890' @@ -28,6 +33,9 @@ if otp_yubikey: INSTALLED_APPS.extend(['otp_yubikey', 'two_factor.plugins.yubikey']) +if webauthn: + INSTALLED_APPS.extend(['two_factor.plugins.webauthn']) + MIDDLEWARE = ( 'django.middleware.common.CommonMiddleware', 'django.contrib.sessions.middleware.SessionMiddleware', @@ -40,6 +48,8 @@ ROOT_URLCONF = 'tests.urls' +STATIC_URL = '/static/' + LOGIN_URL = 'two_factor:login' LOGIN_REDIRECT_URL = 'two_factor:profile' @@ -81,6 +91,8 @@ TWO_FACTOR_PATCH_ADMIN = False +TWO_FACTOR_WEBAUTHN_RP_NAME = 'Test Server' + AUTH_USER_MODEL = os.environ.get('AUTH_USER_MODEL', 'auth.User') PASSWORD_HASHERS = ['django.contrib.auth.hashers.MD5PasswordHasher'] diff --git a/tox.ini b/tox.ini index 6decc5d67..ca35c389b 100644 --- a/tox.ini +++ b/tox.ini @@ -2,9 +2,9 @@ ; Minimum version of Tox minversion = 1.8 envlist = - py{37,38,39,310}-dj32-{normal,yubikey,custom_user}, - py{38,39,310}-dj{40,41}-{normal,yubikey,custom_user} - py{38,39,310}-djmain-{normal,yubikey,custom_user} + py{37,38,39,310}-dj32-{normal,yubikey,custom_user,webauthn} + py{38,39,310}-dj{40,41}-{normal,yubikey,custom_user,webauthn} + py{38,39,310}-djmain-{normal,yubikey,custom_user,webauthn} [gh-actions] python = @@ -21,10 +21,12 @@ DJANGO = main: djmain VARIANT = normal: normal + webauthn: webauthn yubikey: yubikey custom_user: custom_user [testenv] +passenv = HOME DISPLAY setenv = PYTHONDONTWRITEBYTECODE=1 PYTHONWARNINGS=always @@ -40,11 +42,14 @@ deps = dj41: Django<4.2 djmain: https://github.com/django/django/archive/main.tar.gz yubikey: django-otp-yubikey + webauthn: webauthn>=1.2.1,<1.99 + webauthn: -rrequirements_e2e.txt coverage extras = call phonenumberslite yubikey: yubikey + webauthn: webauthn ignore_outcome = djmain: True commands = diff --git a/two_factor/plugins/webauthn/__init__.py b/two_factor/plugins/webauthn/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/two_factor/plugins/webauthn/admin.py b/two_factor/plugins/webauthn/admin.py new file mode 100644 index 000000000..51465c152 --- /dev/null +++ b/two_factor/plugins/webauthn/admin.py @@ -0,0 +1,9 @@ +from django.contrib.admin import ModelAdmin, register + +from .models import WebauthnDevice + + +@register(WebauthnDevice) +class WebauthnDeviceAdmin(ModelAdmin): + list_display = ['user', 'name', 'created_at', 'last_used_at', 'confirmed'] + raw_id_fields = ['user'] diff --git a/two_factor/plugins/webauthn/apps.py b/two_factor/plugins/webauthn/apps.py new file mode 100644 index 000000000..14c53aa67 --- /dev/null +++ b/two_factor/plugins/webauthn/apps.py @@ -0,0 +1,52 @@ +from django.apps import AppConfig +from django.conf import settings +from django.core.exceptions import ImproperlyConfigured + +from two_factor.plugins.registry import registry + + +class TwoFactorWebauthnConfig(AppConfig): + name = 'two_factor.plugins.webauthn' + label = 'two_factor_webauthn' + verbose_name = "Django Two Factor Authentication - WebAuthn Method" + url_prefix = 'webauthn' + + defaults = { + 'TWO_FACTOR_WEBAUTHN_RP_NAME': None, + 'TWO_FACTOR_WEBAUTHN_AUTHENTICATOR_ATTACHMENT': None, + 'TWO_FACTOR_WEBAUTHN_PREFERRED_TRANSPORTS': None, + 'TWO_FACTOR_WEBAUTHN_UV_REQUIREMENT': 'discouraged', + 'TWO_FACTOR_WEBAUTHN_ATTESTATION_CONVEYANCE': 'none', + 'TWO_FACTOR_WEBAUTHN_PEM_ROOT_CERTS_BYTES_BY_FMT': None, + 'TWO_FACTOR_WEBAUTHN_ENTITIES_FORM_MIXIN': + 'two_factor.plugins.webauthn.forms.DefaultWebauthnEntitiesFormMixin', + 'TWO_FACTOR_WEBAUTHN_RP_ID': None, + 'TWO_FACTOR_WEBAUTHN_THROTTLE_FACTOR': 1, + } + + def ready(self): + try: + from webauthn.helpers.structs import ( + AttestationConveyancePreference, + ) + except ImportError: + raise ImproperlyConfigured( + "'webauthn' must be installed to be able to use the webauthn plugin." + ) + + for name, default in self.defaults.items(): + value = getattr(settings, name, default) + setattr(settings, name, value) + + if not settings.TWO_FACTOR_WEBAUTHN_RP_NAME: + raise ImproperlyConfigured('The TWO_FACTOR_WEBAUTHN_RP_NAME setting must not be empty.') + + if settings.TWO_FACTOR_WEBAUTHN_ATTESTATION_CONVEYANCE == AttestationConveyancePreference.ENTERPRISE: + raise ImproperlyConfigured( + f"'{AttestationConveyancePreference.ENTERPRISE}' is not a supported" + " value for TWO_FACTOR_WEBAUTHN_ATTESTATION_CONVEYANCE." + ) + + from .method import WebAuthnMethod + + registry.register(WebAuthnMethod()) diff --git a/two_factor/plugins/webauthn/forms.py b/two_factor/plugins/webauthn/forms.py new file mode 100644 index 000000000..b3cf33e94 --- /dev/null +++ b/two_factor/plugins/webauthn/forms.py @@ -0,0 +1,144 @@ +from hashlib import sha1 + +from django import forms +from django.conf import settings +from django.urls import reverse_lazy +from django.utils import timezone +from django.utils.module_loading import import_string +from django.utils.translation import gettext_lazy as _ +from pydantic.error_wrappers import ValidationError as PydanticValidationError +from webauthn.helpers.exceptions import InvalidAuthenticationResponse +from webauthn.helpers.structs import ( + PublicKeyCredentialRpEntity, PublicKeyCredentialUserEntity, +) + +from two_factor.forms import AuthenticationTokenForm, DeviceValidationForm + +from .models import WebauthnDevice +from .utils import ( + AuthenticationCredential, RegistrationCredential, + make_credential_creation_options, make_credential_request_options, + verify_authentication_response, +) + + +class DefaultWebauthnEntitiesFormMixin: + """ + Mixin to build WebAuthn entities from HttpRequest instances + """ + + @property + def webauthn_user(self): + user = self.request.user + + return PublicKeyCredentialUserEntity( + id=sha1(str(user.pk).encode('utf-8')).hexdigest().encode('utf-8'), + name=user.get_username(), + display_name=user.get_full_name() or user.get_username() + ) + + @property + def webauthn_rp(self): + rp_id = settings.TWO_FACTOR_WEBAUTHN_RP_ID or self.request.get_host().split(':')[0] + + return PublicKeyCredentialRpEntity( + id=rp_id, + name=settings.TWO_FACTOR_WEBAUTHN_RP_NAME, + ) + + @property + def webauthn_origin(self): + scheme = 'https' if self.request.is_secure() else 'http' + return '{scheme}://{host}'.format(scheme=scheme, host=self.request.get_host()) + + +WebauthnEntitiesFormMixin = import_string(settings.TWO_FACTOR_WEBAUTHN_ENTITIES_FORM_MIXIN) + + +class WebauthnAuthenticationTokenForm(WebauthnEntitiesFormMixin, AuthenticationTokenForm): + @property + def media(self): + return forms.Media(js=('two_factor/js/webauthn_utils.js', reverse_lazy('two_factor:webauthn:get_credential'))) + + def __init__(self, user, initial_device, request, **kwargs): + super().__init__(user, initial_device, **kwargs) + self.request = request + + self.fields['otp_token'] = forms.CharField(label=_('Token'), widget=forms.PasswordInput( + attrs={'autofocus': 'autofocus', 'inputmode': 'none', 'autocomplete': 'one-time-code', 'readonly': True})) + if not self.data: + key_handle_allow_list = WebauthnDevice.objects.filter(user=user).values_list('key_handle', flat=True) + options, challenge = make_credential_request_options( + self.webauthn_rp, allowed_credential_ids=key_handle_allow_list) + + self.request.session['webauthn_request_options'] = options + self.request.session['webauthn_request_challenge'] = challenge + + def _verify_token(self, user, token, device=None): + challenge = self.request.session.pop('webauthn_request_challenge') + del self.request.session['webauthn_request_options'] + + try: + credential_id = AuthenticationCredential.parse_raw(token).id + device = WebauthnDevice.objects.get(user=user, key_handle=credential_id) + + new_sign_count = verify_authentication_response( + device.public_key, device.sign_count, self.webauthn_rp, self.webauthn_origin, challenge, token) + except (PydanticValidationError, WebauthnDevice.DoesNotExist, InvalidAuthenticationResponse) as exc: + raise forms.ValidationError(_('Entered token is not valid.'), code='invalid_token') from exc + + device.sign_count = new_sign_count + device.last_used_at = timezone.now() + device.save() + + return device + + def _chosen_device(self, user): + return self.initial_device + + +class WebauthnDeviceValidationForm(WebauthnEntitiesFormMixin, DeviceValidationForm): + token = forms.CharField( + label=_("WebAuthn Token"), + widget=forms.PasswordInput(attrs={ + 'readonly': 'readonly', + 'autocomplete': 'one-time-code', + }) + ) + idempotent = False + + class Media: + js = ('two_factor/js/webauthn_utils.js', reverse_lazy('two_factor:webauthn:create_credential')) + + def __init__(self, device, request, **kwargs): + super().__init__(device, **kwargs) + self.request = request + + if not self.data: + user_key_handles = \ + WebauthnDevice.objects.filter(user=request.user).values_list('key_handle', flat=True) + options, expected_challenge = make_credential_creation_options( + self.webauthn_user, self.webauthn_rp, excluded_credential_ids=user_key_handles) + + self.request.session['webauthn_creation_options'] = options + self.request.session['webauthn_creation_challenge'] = expected_challenge + + def clean_token(self): + expected_challenge = self.request.session['webauthn_creation_challenge'] + token = self.cleaned_data['token'] + + try: + RegistrationCredential.parse_raw(token) + except PydanticValidationError as exc: + raise forms.ValidationError(_('Entered token is not valid.'), code='invalid_token') from exc + + self.cleaned_data = { + **self.cleaned_data, + 'expected_rp_id': self.webauthn_rp.id, + 'expected_origin': self.webauthn_origin, + 'expected_challenge': expected_challenge, + } + + del self.request.session['webauthn_creation_options'] + del self.request.session['webauthn_creation_challenge'] + return token diff --git a/two_factor/plugins/webauthn/method.py b/two_factor/plugins/webauthn/method.py new file mode 100644 index 000000000..37dfad9e8 --- /dev/null +++ b/two_factor/plugins/webauthn/method.py @@ -0,0 +1,65 @@ +from django.utils.translation import gettext_lazy as _ + +from two_factor.plugins.registry import MethodBase + +from .forms import ( + WebauthnAuthenticationTokenForm, WebauthnDeviceValidationForm, +) +from .models import WebauthnDevice +from .utils import verify_registration_response + + +class WebAuthnMethod(MethodBase): + code = 'webauthn' + verbose_name = _('WebAuthn') + + def get_devices(self, user): + return user.webauthn_keys.all() + + def get_other_authentication_devices(self, user, main_device): + # authentication is attempted on all WebAuthn devices at the same time + # if main_device is a WebAuthn device then WebAuthn is the primary method + # and there are no "other" WebAuthn devices + if self.recognize_device(main_device): + return [] + + for device in self.get_devices(user): + # first WebAuthn device found is enough to trigger on all of them at the same time + return [device] + return [] + + def recognize_device(self, device): + return isinstance(device, WebauthnDevice) + + def get_setup_forms(self, *args): + return {self.code: WebauthnDeviceValidationForm} + + def get_device_from_setup_data(self, request, setup_data, **kwargs): + webauthn_setup_data = setup_data.get('webauthn') + if webauthn_setup_data is None: + return None + + expected_rp_id = webauthn_setup_data['expected_rp_id'] + expected_origin = webauthn_setup_data['expected_origin'] + expected_challenge = webauthn_setup_data['expected_challenge'] + token = webauthn_setup_data['token'] + + public_key, key_handle, sign_count = verify_registration_response( + expected_rp_id, expected_origin, expected_challenge, token) + + return WebauthnDevice( + name='default', + public_key=public_key, + key_handle=key_handle, + sign_count=sign_count, + user=request.user + ) + + def get_token_form_class(self): + return WebauthnAuthenticationTokenForm + + def get_action(self, device): + return _('Authenticate using a WebAuthn-compatible device') + + def get_verbose_action(self, device): + return _('Please use your WebAuthn-compatible device to authenticate.') diff --git a/two_factor/plugins/webauthn/migrations/0001_initial.py b/two_factor/plugins/webauthn/migrations/0001_initial.py new file mode 100644 index 000000000..887fe3bea --- /dev/null +++ b/two_factor/plugins/webauthn/migrations/0001_initial.py @@ -0,0 +1,58 @@ +# Generated by Django 3.2.14 on 2022-08-21 02:53 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name='WebauthnDevice', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('name', models.CharField(help_text='The human-readable name of this device.', max_length=64)), + ('confirmed', models.BooleanField(default=True, help_text='Is this device ready for use?')), + ( + 'throttling_failure_timestamp', + models.DateTimeField( + blank=True, + default=None, + help_text='A timestamp of the last failed verification attempt.' + ' Null if last attempt succeeded.', + null=True, + ), + ), + ( + 'throttling_failure_count', + models.PositiveIntegerField( + default=0, + help_text='Number of successive failed attempts.', + ), + ), + ('created_at', models.DateTimeField(auto_now_add=True)), + ('last_used_at', models.DateTimeField(null=True)), + ('public_key', models.TextField(unique=True)), + ('key_handle', models.TextField()), + ('sign_count', models.IntegerField()), + ( + 'user', + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name='webauthn_keys', + to=settings.AUTH_USER_MODEL, + ), + ), + ], + options={ + 'abstract': False, + }, + ), + ] diff --git a/two_factor/plugins/webauthn/migrations/__init__.py b/two_factor/plugins/webauthn/migrations/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/two_factor/plugins/webauthn/models.py b/two_factor/plugins/webauthn/models.py new file mode 100644 index 000000000..9b49f939e --- /dev/null +++ b/two_factor/plugins/webauthn/models.py @@ -0,0 +1,19 @@ +from django.conf import settings +from django.db import models +from django_otp.models import Device, ThrottlingMixin + + +class WebauthnDevice(ThrottlingMixin, Device): + """ + Model for Webauthn authentication + """ + user = models.ForeignKey(settings.AUTH_USER_MODEL, related_name='webauthn_keys', on_delete=models.CASCADE) + created_at = models.DateTimeField(auto_now_add=True) + last_used_at = models.DateTimeField(null=True) + + public_key = models.TextField(unique=True) + key_handle = models.TextField() + sign_count = models.IntegerField() + + def get_throttle_factor(self): + return settings.TWO_FACTOR_WEBAUTHN_THROTTLE_FACTOR diff --git a/two_factor/plugins/webauthn/static/two_factor/js/webauthn_utils.js b/two_factor/plugins/webauthn/static/two_factor/js/webauthn_utils.js new file mode 100644 index 000000000..7756bba7e --- /dev/null +++ b/two_factor/plugins/webauthn/static/two_factor/js/webauthn_utils.js @@ -0,0 +1,21 @@ +function ab2str(buf) { + if (buf == null) { + return null; + }; + + return btoa(String.fromCharCode.apply(null, new Uint8Array(buf))); +} + +function b64str2ab(b64_encoded_string) { + if (b64_encoded_string == null) { + return null; + }; + + let string = atob(b64_encoded_string.replace(/_/g, '/').replace(/-/g, '+')), + buf = new ArrayBuffer(string.length), + bufView = new Uint8Array(buf); + for (var i = 0, strLen = string.length; i < strLen; i++) { + bufView[i] = string.charCodeAt(i); + } + return buf; +} diff --git a/two_factor/plugins/webauthn/templates/two_factor_webauthn/create_credential.js b/two_factor/plugins/webauthn/templates/two_factor_webauthn/create_credential.js new file mode 100644 index 000000000..4acb5c7e8 --- /dev/null +++ b/two_factor/plugins/webauthn/templates/two_factor_webauthn/create_credential.js @@ -0,0 +1,37 @@ +let credentialCreationOptions = {{ credential_creation_options|safe }}; + +credentialCreationOptions.challenge = b64str2ab(credentialCreationOptions.challenge); +for (let i = 0; i < credentialCreationOptions.excludeCredentials.length; i++) { + credentialCreationOptions.excludeCredentials[i].id = b64str2ab(credentialCreationOptions.excludeCredentials[i].id); +} +credentialCreationOptions.user.id = b64str2ab(credentialCreationOptions.user.id); + +navigator.credentials.create({ + publicKey: credentialCreationOptions +}).then((attestationCredential) => { + let response = attestationCredential.response, + serializableAttestationCredential = { + id: attestationCredential.id, + rawId: ab2str(attestationCredential.rawId), + response: { + clientDataJSON: ab2str(response.clientDataJSON), + attestationObject: ab2str(response.attestationObject), + }, + type: attestationCredential.type, + }, + tokenField = document.querySelector('[name=webauthn-token]'), + form = document.forms[0]; + + tokenField.value = JSON.stringify(serializableAttestationCredential); + form.submit(); + +}, (reason) => { + console.debug("Registration error: ", reason); + + let errMsgNode = document.createElement("p"), + tokenField = document.querySelector('#id_webauthn-token'); + + errMsgNode.setAttribute("class", "text-danger"); + errMsgNode.appendChild(document.createTextNode(reason)); + tokenField.parentNode.insertBefore(errMsgNode, tokenField.nextSibling); +}); diff --git a/two_factor/plugins/webauthn/templates/two_factor_webauthn/get_credential.js b/two_factor/plugins/webauthn/templates/two_factor_webauthn/get_credential.js new file mode 100644 index 000000000..7859c7f21 --- /dev/null +++ b/two_factor/plugins/webauthn/templates/two_factor_webauthn/get_credential.js @@ -0,0 +1,37 @@ +let credentialRequestOptions = {{ credential_request_options|safe }}; + +credentialRequestOptions.challenge = b64str2ab(credentialRequestOptions.challenge); +for (let i = 0; i < credentialRequestOptions.allowCredentials.length; i++) { + credentialRequestOptions.allowCredentials[i].id = b64str2ab(credentialRequestOptions.allowCredentials[i].id); +} + +navigator.credentials.get({ + publicKey: credentialRequestOptions +}).then((assertionCredential) => { + let response = assertionCredential.response, + serializableAssertionCredential = { + id: assertionCredential.id, + rawId: ab2str(assertionCredential.rawId), + response: { + clientDataJSON: ab2str(response.clientDataJSON), + authenticatorData: ab2str(response.authenticatorData), + signature: ab2str(response.signature), + userHandle: ab2str(response.userHandle), + }, + type: assertionCredential.type, + }, + tokenField = document.querySelector('[name=token-otp_token]'), + authenticationTokenForm = document.forms[0]; + + tokenField.value = JSON.stringify(serializableAssertionCredential); + authenticationTokenForm.submit(); +}, (reason) => { + console.debug("Authentication error: ", reason); + + let errMsgNode = document.createElement("p"), + tokenField = document.querySelector('[name=token-otp_token]'); + + errMsgNode.setAttribute("class", "text-danger"); + errMsgNode.appendChild(document.createTextNode(reason)); + tokenField.parentNode.insertBefore(errMsgNode, tokenField.nextSibling); +}); diff --git a/two_factor/plugins/webauthn/tests/__init__.py b/two_factor/plugins/webauthn/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/two_factor/plugins/webauthn/tests/test_e2e.py b/two_factor/plugins/webauthn/tests/test_e2e.py new file mode 100644 index 000000000..3be92864a --- /dev/null +++ b/two_factor/plugins/webauthn/tests/test_e2e.py @@ -0,0 +1,114 @@ +try: + from selenium import webdriver + from selenium.webdriver.common.by import By + from selenium.webdriver.support import expected_conditions as EC + from selenium.webdriver.support.ui import WebDriverWait +except ImportError: + webdriver = None + +from unittest import skipUnless +from urllib.parse import urljoin + +from django.contrib.staticfiles.testing import StaticLiveServerTestCase +from django.urls import reverse + +from tests.utils import UserMixin +from two_factor.utils import default_device + +try: + import webauthn +except ImportError: + webauthn = None + +try: + from webdriver_manager.chrome import ChromeDriverManager +except ImportError: + ChromeDriverManager = None + + +@skipUnless(webdriver, 'package selenium is not present') +@skipUnless(ChromeDriverManager, 'package webdriver-manager is not present') +@skipUnless(webauthn, 'package webauthn is not present') +class E2ETests(UserMixin, StaticLiveServerTestCase): + port = 8000 + timeout = 8 + + def setUp(self): + self.base_url = f'http://{self.host}:{self.port}' + self.login_url = urljoin(self.base_url, reverse("two_factor:login")) + + options = webdriver.ChromeOptions() + options.add_argument('headless') + self.webdriver = webdriver.Chrome(ChromeDriverManager().install(), options=options) + + super().setUp() + + def tearDown(self): + self.webdriver.quit() + super().tearDown() + + def setup_virtual_authenticator(self): + self.webdriver.execute_cdp_cmd('WebAuthn.enable', {}) + virtual_authenticator_options = { + 'protocol': 'u2f', + 'transport': 'usb', + } + self.virtual_authenticator = self.webdriver.execute_cdp_cmd( + 'WebAuthn.addVirtualAuthenticator', {'options': virtual_authenticator_options}) + + def wait_for_element(self, selector_type, element): + return WebDriverWait(self.webdriver, self.timeout).until( + EC.presence_of_element_located((selector_type, element)) + ) + + def wait_for_url(self, url): + WebDriverWait(self.webdriver, self.timeout).until(EC.url_to_be(url)) + + def do_login(self): + self.wait_for_url(self.login_url) + + username = self.webdriver.find_element(By.ID, 'id_auth-username') + username.clear() + username.send_keys("bouke@example.com") + + password = self.webdriver.find_element(By.ID, 'id_auth-password') + password.clear() + password.send_keys("secret") + + button_next = self.webdriver.find_element(By.XPATH, "//button[@type='submit']") + button_next.click() + + def register_authenticator(self): + self.wait_for_url(urljoin(self.base_url, reverse("two_factor:setup"))) + self.webdriver.find_element(By.XPATH, "//button[@type='submit']").click() + + self.wait_for_element(By.XPATH, "//input[@value='webauthn']").click() + self.webdriver.find_element(By.XPATH, "//button[@class='btn btn-primary']").click() + + def test_webauthn_attestation_and_assertion(self): + user = self.create_user() + self.setup_virtual_authenticator() + + self.webdriver.get(self.login_url) + self.do_login() + + # register the webauthn authenticator as a second factor + self.register_authenticator() + self.wait_for_url(urljoin(self.base_url, reverse("two_factor:setup_complete"))) + + # log out, log in + self.webdriver.get(urljoin(self.base_url, reverse("logout") + '?next=' + reverse("two_factor:login"))) + self.do_login() + self.wait_for_element( + By.XPATH, "//p[contains(text(), 'Primary method: Authenticate using a WebAuthn-compatible device')]") + + # try registering the same authenticator and fail + # (have to modify the existing authenticator first, so it's no longer the default one) + authenticator = default_device(user) + self.assertIsNotNone(authenticator) + authenticator.name = 'not default anymore' + authenticator.save() + + self.webdriver.get(urljoin(self.base_url, reverse("two_factor:setup"))) + self.register_authenticator() + self.wait_for_element(By.XPATH, "//p[@class='text-danger']") diff --git a/two_factor/plugins/webauthn/tests/test_forms.py b/two_factor/plugins/webauthn/tests/test_forms.py new file mode 100644 index 000000000..0f53e435c --- /dev/null +++ b/two_factor/plugins/webauthn/tests/test_forms.py @@ -0,0 +1,47 @@ +from unittest import skipUnless + +from django.forms import ValidationError +from django.test import RequestFactory, TestCase +from django.urls import reverse + +try: + import webauthn + + from two_factor.plugins.webauthn.forms import ( + WebauthnAuthenticationTokenForm, WebauthnDeviceValidationForm, + ) +except ImportError: + webauthn = None + + +@skipUnless(webauthn, 'package webauthn is not present') +class WebauthnAuthenticationFormTests(TestCase): + def test_verify_token_with_invalid_token(self): + request_factory = RequestFactory() + data = {'otp-token': 'invalid-token'} + request = request_factory.post(reverse('two_factor:login'), data=data) + request.session = { + 'webauthn_request_challenge': 'a-challenge', + 'webauthn_request_options': 'some-options', + } + + form = WebauthnAuthenticationTokenForm(None, None, request, data=data) + + with self.assertRaises(ValidationError) as context: + form._verify_token(None, 'invalid-token') + + self.assertEquals(context.exception.code, 'invalid_token') + + +@skipUnless(webauthn, 'package webauthn is not present') +class WebauthnDeviceValidationFormTests(TestCase): + def test_clean_token_with_invalid_token(self): + request_factory = RequestFactory() + data = {'token': 'invalid-token'} + request = request_factory.post(reverse('two_factor:setup'), data=data) + request.session = {'webauthn_creation_challenge': 'a-challenge'} + + form = WebauthnDeviceValidationForm(None, request, data=data) + + self.assertFalse(form.is_valid()) + self.assertEquals(form.error_messages.keys(), {'invalid_token'}) diff --git a/two_factor/plugins/webauthn/tests/test_utils.py b/two_factor/plugins/webauthn/tests/test_utils.py new file mode 100644 index 000000000..1eaa4616d --- /dev/null +++ b/two_factor/plugins/webauthn/tests/test_utils.py @@ -0,0 +1,61 @@ +import json +from unittest import skipUnless + +from django.test import TestCase + +try: + import webauthn + from webauthn.helpers import bytes_to_base64url + from webauthn.helpers.structs import ( + PublicKeyCredentialRpEntity, PublicKeyCredentialUserEntity, + ) + + from two_factor.plugins.webauthn.utils import ( + make_credential_creation_options, make_credential_request_options, + ) +except ImportError: + webauthn = None + + +@skipUnless(webauthn, 'package webauthn is not present') +class UtilsTests(TestCase): + def setUp(self): + super().setUp() + self.mocked_user = PublicKeyCredentialUserEntity( + id=b'mocked-user-id', name='mocked-username', display_name='Mocked Display Name') + self.mocked_rp = PublicKeyCredentialRpEntity(id='mocked-rp-id', name='mocked-rp-name') + self.mocked_challenge = bytes_to_base64url(b'mocked-challenge') + self.mocked_user_id_b64 = bytes_to_base64url(b'mocked-user-id') + self.mocked_credential_id_b64 = bytes_to_base64url(b'mocked-credential-id') + + def test_make_credential_creation_options(self): + json_options, challenge_b64 = make_credential_creation_options( + self.mocked_user, self.mocked_rp, [self.mocked_credential_id_b64], challenge=self.mocked_challenge) + options = json.loads(json_options) + + self.assertEquals(options['rp'], {'id': self.mocked_rp.id, 'name': self.mocked_rp.name}) + self.assertEquals( + options['user'], + {'id': self.mocked_user_id_b64, 'name': 'mocked-username', 'displayName': 'Mocked Display Name'}, + ) + self.assertEquals(options['challenge'], self.mocked_challenge) + self.assertEquals(options['excludeCredentials'], [{'type': 'public-key', 'id': self.mocked_credential_id_b64}]) + self.assertEquals( + options['authenticatorSelection'], + {'requireResidentKey': False, 'userVerification': 'discouraged'}, + ) + self.assertEquals(options['attestation'], 'none') + self.assertEquals(challenge_b64, self.mocked_challenge) + + def test_make_credential_request_options(self): + json_options, challenge_b64 = make_credential_request_options( + self.mocked_rp, [self.mocked_credential_id_b64], challenge=self.mocked_challenge) + options = json.loads(json_options) + + self.assertEquals(options['rpId'], self.mocked_rp.id) + self.assertEquals(options['challenge'], self.mocked_challenge) + self.assertEquals(len(options['allowCredentials']), 1) + self.assertEquals(options['allowCredentials'][0]['type'], 'public-key') + self.assertEquals(options['allowCredentials'][0]['id'], self.mocked_credential_id_b64) + self.assertEquals(options['userVerification'], 'discouraged') + self.assertEquals(challenge_b64, self.mocked_challenge) diff --git a/two_factor/plugins/webauthn/tests/test_views_setup.py b/two_factor/plugins/webauthn/tests/test_views_setup.py new file mode 100644 index 000000000..f61adee1e --- /dev/null +++ b/two_factor/plugins/webauthn/tests/test_views_setup.py @@ -0,0 +1,60 @@ +from unittest import mock, skipUnless + +from django.test import TestCase +from django.urls import reverse + +from tests.utils import UserMixin + +try: + import webauthn +except ImportError: + webauthn = None + + +class SetupTest(UserMixin, TestCase): + def setUp(self): + super().setUp() + self.user = self.create_user() + self.login_user() + + @skipUnless(webauthn, 'package webauthn is not present') + def test_setup_webauthn(self): + self.assertEqual(0, self.user.webauthn_keys.count()) + + response = self.client.post( + reverse('two_factor:setup'), + data={'setup_view-current_step': 'welcome'}) + self.assertContains(response, 'Method:') + + response = self.client.post( + reverse('two_factor:setup'), + data={'setup_view-current_step': 'method', + 'method-method': 'webauthn'}) + self.assertContains(response, 'Token:') + session = self.client.session + self.assertIn('webauthn_creation_options', session.keys()) + + response = self.client.post( + reverse('two_factor:setup'), + data={'setup_view-current_step': 'webauthn'}) + self.assertEqual(response.context_data['wizard']['form'].errors, + {'token': ['This field is required.']}) + + with mock.patch( + "two_factor.plugins.webauthn.forms.RegistrationCredential.parse_raw" + ), mock.patch( + "two_factor.plugins.webauthn.method.verify_registration_response" + ) as verify_registration_response: + verify_registration_response.return_value = ( + 'mocked_public_key', + 'mocked_credential_id', + 0, + ) + + response = self.client.post( + reverse('two_factor:setup'), + data={'setup_view-current_step': 'webauthn', + 'webauthn-token': 'a_valid_token'}) + + self.assertRedirects(response, reverse('two_factor:setup_complete')) + self.assertEqual(1, self.user.webauthn_keys.count()) diff --git a/two_factor/plugins/webauthn/urls.py b/two_factor/plugins/webauthn/urls.py new file mode 100644 index 000000000..5fcf3cedc --- /dev/null +++ b/two_factor/plugins/webauthn/urls.py @@ -0,0 +1,18 @@ +from django.urls import path + +from .views import CreateCredentialJS, GetCredentialJS + +app_name = 'webauthn' + +urlpatterns = [ + path( + 'create_credential.js', + CreateCredentialJS.as_view(content_type='text/javascript'), + name='create_credential', + ), + path( + 'get_credential.js', + GetCredentialJS.as_view(content_type='text/javascript'), + name='get_credential', + ), +] diff --git a/two_factor/plugins/webauthn/utils.py b/two_factor/plugins/webauthn/utils.py new file mode 100644 index 000000000..63359c4e5 --- /dev/null +++ b/two_factor/plugins/webauthn/utils.py @@ -0,0 +1,142 @@ +from django.conf import settings +from webauthn import ( + generate_authentication_options, generate_registration_options, + options_to_json, + verify_authentication_response as webauthn_verify_authentication_response, + verify_registration_response as webauthn_verify_registration_response, +) +from webauthn.helpers import base64url_to_bytes, bytes_to_base64url +from webauthn.helpers.structs import ( + AttestationConveyancePreference, AuthenticationCredential, + AuthenticatorAttachment, AuthenticatorSelectionCriteria, + AuthenticatorTransport, PublicKeyCredentialDescriptor, + RegistrationCredential, UserVerificationRequirement, +) + + +def make_credential_creation_options(user, rp, excluded_credential_ids, challenge=None): + """ + Builds the options object needed for `navigator.credentials.create` + to create a public key credential and register a new authenticator + :param user: a PublicKeyCredentialUserEntity instance representing the user will register a new authenticator + :param rp: a PublicKeyCredentialRpEntity instance representing the Relying Party + :param excluded_credential_ids: a list of credential ids of authenticators already registered by this user + :param challenge: the challenge that will be compared to the one returned in the client's response + :return: a JSON-serialized PublicKeyCredentialCreationOptions object + """ + exclude_credentials = [ + PublicKeyCredentialDescriptor(id=base64url_to_bytes(credential_id)) + for credential_id in excluded_credential_ids + ] + if challenge: + challenge = base64url_to_bytes(challenge) + + authenticator_attachment = None + if settings.TWO_FACTOR_WEBAUTHN_AUTHENTICATOR_ATTACHMENT: + authenticator_attachment = AuthenticatorAttachment(settings.TWO_FACTOR_WEBAUTHN_AUTHENTICATOR_ATTACHMENT) + + creation_options = generate_registration_options( + rp_id=rp.id, + rp_name=rp.name, + user_id=user.id.decode('utf-8'), + user_name=user.name, + user_display_name=user.display_name, + challenge=challenge, + attestation=AttestationConveyancePreference(settings.TWO_FACTOR_WEBAUTHN_ATTESTATION_CONVEYANCE), + authenticator_selection=AuthenticatorSelectionCriteria( + authenticator_attachment=authenticator_attachment, + user_verification=UserVerificationRequirement(settings.TWO_FACTOR_WEBAUTHN_UV_REQUIREMENT) + ), + exclude_credentials=exclude_credentials, + ) + return options_to_json(creation_options), bytes_to_base64url(creation_options.challenge) + + +def verify_registration_response(expected_rp_id, expected_origin, expected_challenge, registration_token): + """ + Validate the result of `navigator.credentials.create` + :param expected_rp_id: expected ID of the Relying Party + :param expected_origin: the base domain with protocol on which the registration ceremony took place + :param expected_challenge: the challenge returned by make_credential_creation_options + :param registration_token: a serialized RegistrationCredential object + :return: a tuple with the credential public key, id and current sign count + """ + verified_registration = webauthn_verify_registration_response( + credential=RegistrationCredential.parse_raw(registration_token), + expected_challenge=base64url_to_bytes(expected_challenge), + expected_origin=expected_origin, + expected_rp_id=expected_rp_id, + require_user_verification=settings.TWO_FACTOR_WEBAUTHN_UV_REQUIREMENT == UserVerificationRequirement.REQUIRED, + pem_root_certs_bytes_by_fmt=settings.TWO_FACTOR_WEBAUTHN_PEM_ROOT_CERTS_BYTES_BY_FMT, + ) + + return ( + bytes_to_base64url(verified_registration.credential_public_key), + bytes_to_base64url(verified_registration.credential_id), + verified_registration.sign_count + ) + + +def make_credential_request_options(rp, allowed_credential_ids, challenge=None): + """ + Build the options object needed for `navigator.credentials.get` + to get a credential identifying a user that logged in with a WebAuthn device + :param relying_party: a PublicKeyCredentialRpEntity instance representing the Relying Party + :param allowed_credential_ids: a list of credential ids of authenticators already registered by this user + :param challenge: the challenge that will be compared to the one returned in the client's response + :return: A JSON-serialized PublicKeyCredentialRequestOptions object + """ + preferred_transports = None + if settings.TWO_FACTOR_WEBAUTHN_PREFERRED_TRANSPORTS: + preferred_transports = [ + AuthenticatorTransport(transport) for transport in settings.TWO_FACTOR_WEBAUTHN_PREFERRED_TRANSPORTS + ] + + allow_credentials = [ + PublicKeyCredentialDescriptor( + id=base64url_to_bytes(credential_id), + transports=preferred_transports, + ) + for credential_id in allowed_credential_ids + ] + if challenge: + challenge = base64url_to_bytes(challenge) + + request_options = generate_authentication_options( + rp_id=rp.id, + challenge=challenge, + allow_credentials=allow_credentials, + user_verification=UserVerificationRequirement(settings.TWO_FACTOR_WEBAUTHN_UV_REQUIREMENT), + ) + return options_to_json(request_options), bytes_to_base64url(request_options.challenge) + + +def verify_authentication_response( + public_key, + current_sign_count, + expected_rp, + expected_origin, + expected_challenge, + authentication_token, +): + """ + Validate the result of `navigator.credentials.get` + :public_key: the public key of the credential + :current_sign_count: the current sign count of the credential + :param expected_rp: the expected WebAuthn Relying Party information contained in the credential + :param expected_origin: the base domain with protocol on which the authentication ceremony took place + :param expected_challenge: the challenge returned by make_credential_request_options + :param authentication_token: a serialized AuthenticationCredential object + :return: the new sign count for the WebauthnDevice instance + """ + verified_authentication = webauthn_verify_authentication_response( + credential=AuthenticationCredential.parse_raw(authentication_token), + expected_challenge=base64url_to_bytes(expected_challenge), + expected_rp_id=expected_rp.id, + expected_origin=expected_origin, + credential_public_key=base64url_to_bytes(public_key), + credential_current_sign_count=current_sign_count, + require_user_verification=settings.TWO_FACTOR_WEBAUTHN_UV_REQUIREMENT == UserVerificationRequirement.REQUIRED, + ) + + return verified_authentication.new_sign_count diff --git a/two_factor/plugins/webauthn/views.py b/two_factor/plugins/webauthn/views.py new file mode 100644 index 000000000..b2c211810 --- /dev/null +++ b/two_factor/plugins/webauthn/views.py @@ -0,0 +1,43 @@ +from django.contrib.auth.decorators import login_required +from django.http.response import Http404 +from django.utils.decorators import method_decorator +from django.views.decorators.cache import never_cache +from django.views.generic import TemplateView + + +@method_decorator(never_cache, name='dispatch') +class DynamicJS(TemplateView): + def get_extra_context_data(self): + raise NotImplementedError() + + def get_context_data(self, *args, **kwargs): + extra_context = self.get_extra_context_data() + if not extra_context: + raise Http404() + + context = super().get_context_data(*args, **kwargs) + context.update(extra_context) + + return context + + +@method_decorator(login_required, name='dispatch') +class CreateCredentialJS(DynamicJS): + template_name = 'two_factor_webauthn/create_credential.js' + + def get_extra_context_data(self): + credential_creation_options = self.request.session.get( + 'webauthn_creation_options') + if credential_creation_options: + return {'credential_creation_options': credential_creation_options} + return None + + +class GetCredentialJS(DynamicJS): + template_name = 'two_factor_webauthn/get_credential.js' + + def get_extra_context_data(self): + credential_request_options = self.request.session.get('webauthn_request_options') + if credential_request_options: + return {'credential_request_options': credential_request_options} + return None diff --git a/two_factor/templates/two_factor/_base.html b/two_factor/templates/two_factor/_base.html index 694a32c0a..d11c3c08f 100644 --- a/two_factor/templates/two_factor/_base.html +++ b/two_factor/templates/two_factor/_base.html @@ -6,6 +6,7 @@ + {% block extra_media %}{% endblock %}

    Provide a template named diff --git a/two_factor/templates/two_factor/core/login.html b/two_factor/templates/two_factor/core/login.html index a52ada14b..95789bade 100644 --- a/two_factor/templates/two_factor/core/login.html +++ b/two_factor/templates/two_factor/core/login.html @@ -2,6 +2,10 @@ {% load i18n %} {% load two_factor_tags %} +{% block extra_media %} + {{ form.media }} +{% endblock %} + {% block content %}

    {% block title %}{% trans "Login" %}{% endblock %}

    diff --git a/two_factor/templates/two_factor/core/setup.html b/two_factor/templates/two_factor/core/setup.html index 1064061cb..222039094 100644 --- a/two_factor/templates/two_factor/core/setup.html +++ b/two_factor/templates/two_factor/core/setup.html @@ -1,6 +1,10 @@ {% extends "two_factor/_base_focus.html" %} {% load i18n %} +{% block extra_media %} + {{ form.media }} +{% endblock %} + {% block content %}

    {% block title %}{% trans "Enable Two-Factor Authentication" %}{% endblock %}

    {% if wizard.steps.current == 'welcome' %} diff --git a/two_factor/templatetags/__init__.py b/two_factor/templatetags/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/two_factor/urls.py b/two_factor/urls.py index b612223e8..3962e2912 100644 --- a/two_factor/urls.py +++ b/two_factor/urls.py @@ -1,4 +1,5 @@ -from django.urls import path +from django.apps.registry import apps +from django.urls import include, path from two_factor.plugins.phonenumber.views import ( PhoneDeleteView, PhoneSetupView, @@ -59,4 +60,17 @@ ), ] -urlpatterns = (core + profile, 'two_factor') +plugin_urlpatterns = [] +for app_config in apps.get_app_configs(): + if app_config.name.startswith('two_factor.plugins.'): + try: + plugin_urlpatterns.append( + path( + f'account/two_factor/{app_config.url_prefix}/', + include(f'{app_config.name}.urls', app_config.label) + ), + ) + except AttributeError: + pass + +urlpatterns = (core + profile + plugin_urlpatterns, 'two_factor') diff --git a/two_factor/views/core.py b/two_factor/views/core.py index 1defd0fe5..1a3ca96c4 100644 --- a/two_factor/views/core.py +++ b/two_factor/views/core.py @@ -133,6 +133,7 @@ def post(self, *args, **kwargs): # Generating a challenge doesn't require to validate the form. if 'challenge_device' in self.request.POST: + self.storage.data['challenge_device'] = self.request.POST['challenge_device'] return self.render_goto_step('token') response = super().post(*args, **kwargs) @@ -276,7 +277,10 @@ def get_device(self, step=None): Returns the OTP device selected by the user, or his default device. """ if not self.device_cache: - challenge_device_id = self.request.POST.get('challenge_device', None) + challenge_device_id = ( + self.request.POST.get('challenge_device') + or self.storage.data.get('challenge_device') + ) if challenge_device_id: for device in self.get_devices(): if device.persistent_id == challenge_device_id: @@ -529,8 +533,9 @@ def done(self, form_list, **kwargs): if method.code == 'generator': form = [form for form in form_list if isinstance(form, TOTPDeviceForm)][0] device = form.save() - # PhoneNumberForm / YubiKeyDeviceForm / EmailForm - elif method.code in ('call', 'sms', 'yubikey', 'email'): + + # PhoneNumberForm / YubiKeyDeviceForm / EmailForm / WebauthnDeviceValidationForm + elif method.code in ('call', 'sms', 'yubikey', 'email', 'webauthn'): device = self.get_device() device.save() From c086b7fd6b58262d9aa95dfcd1c1e0cee841485d Mon Sep 17 00:00:00 2001 From: Javier Paniagua Date: Tue, 27 Sep 2022 10:20:37 +0200 Subject: [PATCH 22/24] use assertEqual instead of deprecated assertEquals --- .../plugins/webauthn/tests/test_forms.py | 4 +-- .../plugins/webauthn/tests/test_utils.py | 28 +++++++++---------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/two_factor/plugins/webauthn/tests/test_forms.py b/two_factor/plugins/webauthn/tests/test_forms.py index 0f53e435c..7afadb3fc 100644 --- a/two_factor/plugins/webauthn/tests/test_forms.py +++ b/two_factor/plugins/webauthn/tests/test_forms.py @@ -30,7 +30,7 @@ def test_verify_token_with_invalid_token(self): with self.assertRaises(ValidationError) as context: form._verify_token(None, 'invalid-token') - self.assertEquals(context.exception.code, 'invalid_token') + self.assertEqual(context.exception.code, 'invalid_token') @skipUnless(webauthn, 'package webauthn is not present') @@ -44,4 +44,4 @@ def test_clean_token_with_invalid_token(self): form = WebauthnDeviceValidationForm(None, request, data=data) self.assertFalse(form.is_valid()) - self.assertEquals(form.error_messages.keys(), {'invalid_token'}) + self.assertEqual(form.error_messages.keys(), {'invalid_token'}) diff --git a/two_factor/plugins/webauthn/tests/test_utils.py b/two_factor/plugins/webauthn/tests/test_utils.py index 1eaa4616d..4fe4a8393 100644 --- a/two_factor/plugins/webauthn/tests/test_utils.py +++ b/two_factor/plugins/webauthn/tests/test_utils.py @@ -33,29 +33,29 @@ def test_make_credential_creation_options(self): self.mocked_user, self.mocked_rp, [self.mocked_credential_id_b64], challenge=self.mocked_challenge) options = json.loads(json_options) - self.assertEquals(options['rp'], {'id': self.mocked_rp.id, 'name': self.mocked_rp.name}) - self.assertEquals( + self.assertEqual(options['rp'], {'id': self.mocked_rp.id, 'name': self.mocked_rp.name}) + self.assertEqual( options['user'], {'id': self.mocked_user_id_b64, 'name': 'mocked-username', 'displayName': 'Mocked Display Name'}, ) - self.assertEquals(options['challenge'], self.mocked_challenge) - self.assertEquals(options['excludeCredentials'], [{'type': 'public-key', 'id': self.mocked_credential_id_b64}]) - self.assertEquals( + self.assertEqual(options['challenge'], self.mocked_challenge) + self.assertEqual(options['excludeCredentials'], [{'type': 'public-key', 'id': self.mocked_credential_id_b64}]) + self.assertEqual( options['authenticatorSelection'], {'requireResidentKey': False, 'userVerification': 'discouraged'}, ) - self.assertEquals(options['attestation'], 'none') - self.assertEquals(challenge_b64, self.mocked_challenge) + self.assertEqual(options['attestation'], 'none') + self.assertEqual(challenge_b64, self.mocked_challenge) def test_make_credential_request_options(self): json_options, challenge_b64 = make_credential_request_options( self.mocked_rp, [self.mocked_credential_id_b64], challenge=self.mocked_challenge) options = json.loads(json_options) - self.assertEquals(options['rpId'], self.mocked_rp.id) - self.assertEquals(options['challenge'], self.mocked_challenge) - self.assertEquals(len(options['allowCredentials']), 1) - self.assertEquals(options['allowCredentials'][0]['type'], 'public-key') - self.assertEquals(options['allowCredentials'][0]['id'], self.mocked_credential_id_b64) - self.assertEquals(options['userVerification'], 'discouraged') - self.assertEquals(challenge_b64, self.mocked_challenge) + self.assertEqual(options['rpId'], self.mocked_rp.id) + self.assertEqual(options['challenge'], self.mocked_challenge) + self.assertEqual(len(options['allowCredentials']), 1) + self.assertEqual(options['allowCredentials'][0]['type'], 'public-key') + self.assertEqual(options['allowCredentials'][0]['id'], self.mocked_credential_id_b64) + self.assertEqual(options['userVerification'], 'discouraged') + self.assertEqual(challenge_b64, self.mocked_challenge) From dffe9a1625f630b683eca8295eb633d20b96975d Mon Sep 17 00:00:00 2001 From: Javier Paniagua Date: Tue, 27 Sep 2022 16:46:03 +0200 Subject: [PATCH 23/24] bump up selenium to fix unclosed socket warning --- requirements_e2e.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/requirements_e2e.txt b/requirements_e2e.txt index bd8c98491..9077a3c2e 100644 --- a/requirements_e2e.txt +++ b/requirements_e2e.txt @@ -1,3 +1,3 @@ # test with selenium -selenium~=3.141.0 -webdriver-manager~=3.4.2 +selenium~=4.4.3 +webdriver-manager~=3.8.3 From cf4b191144e4dcd3268835356a39eef5821f2f0c Mon Sep 17 00:00:00 2001 From: Darrel O'Pry Date: Thu, 5 May 2022 15:13:50 -0400 Subject: [PATCH 24/24] feat: show secret_key with QR code - streamline the setup experience for users working with password managers that cannot read QR codes easily. Co-authored-by: Claude Paroz --- CHANGELOG.md | 5 +++++ tests/test_views_setup.py | 6 ++++++ two_factor/templates/two_factor/core/setup.html | 8 ++++++-- two_factor/views/core.py | 8 ++++++++ 4 files changed, 25 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bde04afb2..ecd8b4ce1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,11 @@ ### Removed - Django 2.2, 3.0, and 3.1 support +### Changed + +- display the TOTP secret key alongside the QR code to streamline setup for + password managers without QR support. + ## 1.14.0 ### Added diff --git a/tests/test_views_setup.py b/tests/test_views_setup.py index f6573c45f..06b9bb3f5 100644 --- a/tests/test_views_setup.py +++ b/tests/test_views_setup.py @@ -38,6 +38,12 @@ def test_setup_only_generator_available(self): # test if secret key is valid base32 and has the correct number of bytes secret_key = response.context_data['secret_key'] self.assertEqual(len(b32decode(secret_key)), 20) + self.assertEqual( + response.context_data['otpauth_url'], + f'otpauth://totp/testserver%3A%20bouke%40example.com?secret={secret_key}&digits=6&issuer=testserver' + ) + self.assertEqual(response.context_data['issuer'], 'testserver') + self.assertEqual(response.context_data['totp_digits'], 6) response = self.client.post( reverse('two_factor:setup'), diff --git a/two_factor/templates/two_factor/core/setup.html b/two_factor/templates/two_factor/core/setup.html index 222039094..1596fee00 100644 --- a/two_factor/templates/two_factor/core/setup.html +++ b/two_factor/templates/two_factor/core/setup.html @@ -17,9 +17,13 @@

    {% block title %}{% trans "Enable Two-Factor Authentication" %}{% endblock % {% elif wizard.steps.current == 'generator' %}

    {% blocktrans trimmed %}To start using a token generator, please use your smartphone to scan the QR code below. For example, use Google - Authenticator. Then, enter the token generated by the app. - {% endblocktrans %}

    + Authenticator.{% endblocktrans %}

    QR Code

    +

    {% blocktrans trimmed %}Alternatively you can use the following secret to + setup TOTP in your authenticator or password manager manually.{% endblocktrans %}

    +

    {% translate "TOTP Secret:" %} {{ secret_key }}

    +

    {% blocktrans %}Then, enter the token generated by the app.{% endblocktrans %}

    + {% elif wizard.steps.current == 'sms' %}

    {% blocktrans trimmed %}Please enter the phone number you wish to receive the text messages on. This number will be validated in the next step. diff --git a/two_factor/views/core.py b/two_factor/views/core.py index 1a3ca96c4..d347e8273 100644 --- a/two_factor/views/core.py +++ b/two_factor/views/core.py @@ -596,10 +596,18 @@ def get_context_data(self, form, **kwargs): key = self.get_key('generator') rawkey = unhexlify(key.encode('ascii')) b32key = b32encode(rawkey).decode('utf-8') + issuer = get_current_site(self.request).name + username = self.request.user.get_username() + otpauth_url = get_otpauth_url(username, b32key, issuer) self.request.session[self.session_key_name] = b32key context.update({ + # used in default template + 'otpauth_url': otpauth_url, 'QR_URL': reverse(self.qrcode_url), 'secret_key': b32key, + # available for custom templates + 'issuer': issuer, + 'totp_digits': totp_digits(), }) elif self.steps.current == 'validation': context['device'] = self.get_device()