diff --git a/corehq/apps/domain/deletion.py b/corehq/apps/domain/deletion.py index 38b047e16cb8..d3c241c12f02 100644 --- a/corehq/apps/domain/deletion.py +++ b/corehq/apps/domain/deletion.py @@ -301,7 +301,7 @@ def _delete_demo_user_restores(domain_name): DOMAIN_DELETE_OPERATIONS = [ DjangoUserRelatedModelDeletion('otp_static', 'StaticDevice', 'user__username', ['StaticToken']), DjangoUserRelatedModelDeletion('otp_totp', 'TOTPDevice', 'user__username'), - DjangoUserRelatedModelDeletion('two_factor', 'PhoneDevice', 'user__username'), + DjangoUserRelatedModelDeletion('phonenumber', 'PhoneDevice', 'user__username'), DjangoUserRelatedModelDeletion('users', 'HQApiKey', 'user__username'), CustomDeletion('auth', _delete_django_users, ['User']), ModelDeletion('products', 'SQLProduct', 'domain'), diff --git a/corehq/apps/domain/templates/login_and_password/two_factor/core/login.html b/corehq/apps/domain/templates/login_and_password/two_factor/core/login.html index 61887c4d673a..102481acf529 100644 --- a/corehq/apps/domain/templates/login_and_password/two_factor/core/login.html +++ b/corehq/apps/domain/templates/login_and_password/two_factor/core/login.html @@ -1,4 +1,4 @@ -{% load i18n two_factor %} +{% load i18n phonenumber %} {% block focus-content %}

diff --git a/corehq/apps/domain/templates/login_and_password/two_factor/core/login_form.html b/corehq/apps/domain/templates/login_and_password/two_factor/core/login_form.html index 92beadb3c119..7fd85eb8a0f9 100644 --- a/corehq/apps/domain/templates/login_and_password/two_factor/core/login_form.html +++ b/corehq/apps/domain/templates/login_and_password/two_factor/core/login_form.html @@ -1,18 +1,18 @@ -{% load i18n two_factor %} +{% load i18n two_factor_tags %}
{% csrf_token %} {% include "two_factor/_wizard_forms.html" %} {# hidden submit button to enable [enter] key #}
- {% if other_devices %} + {% if device.name == "default" and other_devices %}

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

{% for other in other_devices %}

{% endfor %} diff --git a/corehq/apps/domain/templates/login_and_password/two_factor/core/phone_register.html b/corehq/apps/domain/templates/login_and_password/two_factor/core/phone_register.html index dd334433ebac..8739ecf73720 100644 --- a/corehq/apps/domain/templates/login_and_password/two_factor/core/phone_register.html +++ b/corehq/apps/domain/templates/login_and_password/two_factor/core/phone_register.html @@ -1,3 +1,4 @@ +{# lightly modified version of two_factor/core/phone_register.html #} {% extends "hqwebapp/bootstrap3/base_section.html" %} {% load i18n %} {% load crispy_forms_tags %} @@ -5,11 +6,11 @@ {% block page_content %}

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

- {% if wizard.steps.current == 'method' %} -

{% blocktrans %}Your backup phone number will be used if your primary method of + {% if wizard.steps.current == 'setup' %} +

{% blocktrans trimmed %}Your backup phone number will be used if your primary method of registration is not available. Please enter a valid phone number.{% endblocktrans %}

{% elif wizard.steps.current == 'validation' %} -

{% blocktrans %}We've sent a token to your phone number. Please +

{% blocktrans trimmed %}We've sent a token to your phone number. Please enter the token you've received.{% endblocktrans %}

{% endif %} diff --git a/corehq/apps/domain/templates/login_and_password/two_factor/core/setup.html b/corehq/apps/domain/templates/login_and_password/two_factor/core/setup.html index 0e7c716d3352..99eb4c3fdb51 100644 --- a/corehq/apps/domain/templates/login_and_password/two_factor/core/setup.html +++ b/corehq/apps/domain/templates/login_and_password/two_factor/core/setup.html @@ -1,41 +1,51 @@ +{# lightly modified version of two_factor/core/setup.html #} {% extends "hqwebapp/bootstrap3/base_section.html" %} {% load i18n %} {% load crispy_forms_tags %} +{% block extra_media %} + {{ form.media }} +{% endblock %} + {% block page_content %}

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

- {% if wizard.steps.current == 'welcome_setup' %} -

{% blocktrans %}Follow the steps in this wizard to enable two-factor - authentication.{% endblocktrans %}

- {% elif wizard.steps.current == 'welcome_reset' %} -

{% blocktrans %}Follow the steps in this wizard to reset two-factor - authentication.{% endblocktrans %}

+ {% if wizard.steps.current == 'welcome' %} +

{% blocktrans trimmed %}Follow the steps in this wizard to enable two-factor + authentication.{% endblocktrans %}

{% elif wizard.steps.current == 'method' %} -

{% blocktrans %}Please select which authentication method you would - like to use.{% endblocktrans %}

+

{% blocktrans trimmed %}Please select which authentication method you would + like to use.{% endblocktrans %}

{% elif wizard.steps.current == 'generator' %} -

{% blocktrans %}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 %}

-

QR Code

+

{% 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 %}

+

QR Code

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

{% blocktrans %}Please enter the phone number you wish to receive the +

{% blocktrans trimmed %}Please enter the phone number you wish to receive the text messages on. This number will be validated in the next step. - {% endblocktrans %}

+ {% endblocktrans %}

{% elif wizard.steps.current == 'call' %} -

{% blocktrans %}Please enter the phone number you wish to be called on. +

{% blocktrans trimmed %}Please enter the phone number you wish to be called on. This number will be validated in the next step. {% endblocktrans %}

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

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

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

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

+ {% if challenge_succeeded %} + {% 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 %}

+ {% endif %} + {% else %} + {% endif %} {% elif wizard.steps.current == 'yubikey' %} -

{% blocktrans %}To identify and verify your YubiKey, please insert a +

{% blocktrans trimmed %}To identify and verify your YubiKey, please insert a token in the field below. Your YubiKey will be linked to your account.{% endblocktrans %}

{% endif %} diff --git a/corehq/apps/domain/templates/login_and_password/two_factor/profile/profile.html b/corehq/apps/domain/templates/login_and_password/two_factor/profile/profile.html index 7f246170a6cb..20ab88e073ce 100644 --- a/corehq/apps/domain/templates/login_and_password/two_factor/profile/profile.html +++ b/corehq/apps/domain/templates/login_and_password/two_factor/profile/profile.html @@ -1,5 +1,5 @@ {% extends "hqwebapp/bootstrap3/base_section.html" %} -{% load i18n two_factor %} +{% load i18n phonenumber %} {% block page_content %} {% if is_using_sso %} @@ -59,8 +59,8 @@
{% trans "Remove Two-Factor Authentication" %} -

{% blocktrans %}However strongly we discourage you to do so, you can - also remove two-factor authentication for your account.{% endblocktrans %}

+

{% blocktrans %}We strongly discourage this, but if absolutely necessary you can + remove two-factor authentication from your account.{% endblocktrans %}

{% trans "Remove Two-Factor Authentication" %}


@@ -68,8 +68,9 @@
{% trans "Reset Two-Factor Authentication" %}

{% blocktrans %} - Clicking below will disable your current two-factor authentication and rerun Two-Factor Authentication setup, - so make sure you have a backup device or tokens available in case you are unable to complete the process. + This will remove your current two-factor authentication, and prompt you to run through the entire setup again. + If you need to do this, please make sure you complete the entire process once you begin, otherwise your account will + not be protected by two-factor authentication. {% endblocktrans %}

{% trans "Reset Two-Factor Authentication" %}

diff --git a/corehq/apps/dump_reload/tests/test_dump_models.py b/corehq/apps/dump_reload/tests/test_dump_models.py index 3d2808404193..b2b9eeeae53c 100644 --- a/corehq/apps/dump_reload/tests/test_dump_models.py +++ b/corehq/apps/dump_reload/tests/test_dump_models.py @@ -94,7 +94,7 @@ "tastypie.ApiAccess", # not tagged by domain "tastypie.ApiKey", # not domain-specific "toggle_ui.ToggleAudit", - "two_factor.PhoneDevice", + "phonenumber.PhoneDevice", "users.Permission", "util.BouncedEmail", "util.ComplaintBounceMeta", diff --git a/corehq/apps/hqwebapp/apps.py b/corehq/apps/hqwebapp/apps.py index f9f210aaf75f..7027388964c4 100644 --- a/corehq/apps/hqwebapp/apps.py +++ b/corehq/apps/hqwebapp/apps.py @@ -1,4 +1,7 @@ from django.apps import AppConfig +from django.conf import settings + +from two_factor.plugins.registry import registry class HqWebAppConfig(AppConfig): @@ -7,3 +10,34 @@ class HqWebAppConfig(AppConfig): def ready(self): # Ensure the login signal handlers have been loaded from . import login_handlers, signals # noqa + + # custom 2FA methods need to be registered on startup + register_two_factor_methods() + + +def register_two_factor_methods(): + from corehq.apps.hqwebapp.two_factor_methods import ( + HQGeneratorMethod, + HQPhoneCallMethod, + HQSMSMethod, + ) + + # default generator method is registered when the registry object is created + # https://github.com/jazzband/django-two-factor-auth/blob/1.15.5/two_factor/plugins/registry.py#L76-L77 + registry.unregister('generator') + registry.register(HQGeneratorMethod()) + + # default phone methods are registered when django starts up + # https://github.com/jazzband/django-two-factor-auth/blob/1.15.5/two_factor/plugins/phonenumber/apps.py#L19-L30 + if not settings.ALLOW_PHONE_AS_DEFAULT_TWO_FACTOR_DEVICE: + registry.unregister('call') + registry.unregister('sms') + return + + if getattr(settings, 'TWO_FACTOR_CALL_GATEWAY', None): + registry.unregister('call') + registry.register(HQPhoneCallMethod()) + + if getattr(settings, 'TWO_FACTOR_SMS_GATEWAY', None): + registry.unregister('sms') + registry.register(HQSMSMethod()) diff --git a/corehq/apps/hqwebapp/decorators.py b/corehq/apps/hqwebapp/decorators.py index af7cac42c5a9..5e80500e6b34 100644 --- a/corehq/apps/hqwebapp/decorators.py +++ b/corehq/apps/hqwebapp/decorators.py @@ -1,8 +1,6 @@ from collections import defaultdict from functools import wraps -from django.urls import get_resolver - from corehq.apps.hqwebapp.utils.bootstrap import set_bootstrap_version5 diff --git a/corehq/apps/hqwebapp/tests/test_apps.py b/corehq/apps/hqwebapp/tests/test_apps.py new file mode 100644 index 000000000000..2b53680f9bd0 --- /dev/null +++ b/corehq/apps/hqwebapp/tests/test_apps.py @@ -0,0 +1,46 @@ +from django.test import SimpleTestCase, override_settings + +from two_factor.plugins.registry import registry + +from corehq.apps.hqwebapp.apps import register_two_factor_methods +from corehq.apps.hqwebapp.two_factor_methods import ( + HQGeneratorMethod, + HQPhoneCallMethod, + HQSMSMethod, +) + + +@override_settings( + ALLOW_PHONE_AS_DEFAULT_TWO_FACTOR_DEVICE=True, + TWO_FACTOR_SMS_GATEWAY='corehq.apps.hqwebapp.two_factor_gateways.Gateway', + TWO_FACTOR_CALL_GATEWAY='corehq.apps.hqwebapp.two_factor_gateways.Gateway', +) +class RegisterCustom2FAMethodsTests(SimpleTestCase): + + @override_settings( + ALLOW_PHONE_AS_DEFAULT_TWO_FACTOR_DEVICE=False, + TWO_FACTOR_SMS_GATEWAY=None, + TWO_FACTOR_CALL_GATEWAY=None, + ) + def test_custom_generator_method_is_registered(self): + register_two_factor_methods() + method = registry.get_method('generator') + self.assertIsInstance(method, HQGeneratorMethod) + + @override_settings(TWO_FACTOR_SMS_GATEWAY=None) + def test_custom_call_method_is_registered_if_relevant_settings_are_true(self): + register_two_factor_methods() + method = registry.get_method('call') + self.assertIsInstance(method, HQPhoneCallMethod) + + @override_settings(TWO_FACTOR_CALL_GATEWAY=None) + def test_custom_sms_method_is_registered_if_relevant_settings_are_true(self): + register_two_factor_methods() + method = registry.get_method('sms') + self.assertIsInstance(method, HQSMSMethod) + + @override_settings(ALLOW_PHONE_AS_DEFAULT_TWO_FACTOR_DEVICE=False) + def test_neither_phone_method_is_registered_if_allow_phone_setting_is_false(self): + register_two_factor_methods() + self.assertIsNone(registry.get_method('call')) + self.assertIsNone(registry.get_method('sms')) diff --git a/corehq/apps/hqwebapp/two_factor_gateways.py b/corehq/apps/hqwebapp/two_factor_gateways.py index 2891f444dc00..69ff68b8d28b 100644 --- a/corehq/apps/hqwebapp/two_factor_gateways.py +++ b/corehq/apps/hqwebapp/two_factor_gateways.py @@ -13,7 +13,7 @@ from twilio.base.exceptions import TwilioRestException from twilio.http.http_client import TwilioHttpClient from twilio.rest import Client -from two_factor.models import PhoneDevice +from two_factor.plugins.phonenumber.models import PhoneDevice import settings from corehq.apps.users.models import CouchUser diff --git a/corehq/apps/hqwebapp/two_factor_methods.py b/corehq/apps/hqwebapp/two_factor_methods.py new file mode 100644 index 000000000000..5ca4ca1bf68a --- /dev/null +++ b/corehq/apps/hqwebapp/two_factor_methods.py @@ -0,0 +1,40 @@ +from django_otp.plugins.otp_totp.models import TOTPDevice + +from two_factor.plugins.phonenumber.method import PhoneCallMethod, SMSMethod +from two_factor.plugins.registry import GeneratorMethod + +from corehq.apps.hqwebapp.forms import HQAuthenticationTokenForm +from corehq.apps.settings.forms import HQPhoneNumberForm, HQTOTPDeviceForm + + +class HQGeneratorMethod(GeneratorMethod): + + # only overriding this because it is set in GeneratorMethod + form_path = 'corehq.apps.settings.forms.HQTOTPDeviceForm' + + def get_setup_forms(self, *args): + return {self.code: HQTOTPDeviceForm} + + def get_token_form_class(self): + return HQAuthenticationTokenForm + + def recognize_device(self, device): + return isinstance(device, TOTPDevice) + + +class HQPhoneCallMethod(PhoneCallMethod): + + def get_setup_forms(self, *args): + return {self.code: HQPhoneNumberForm} + + def get_token_form_class(self): + return HQAuthenticationTokenForm + + +class HQSMSMethod(SMSMethod): + + def get_setup_forms(self, *args): + return {self.code: HQPhoneNumberForm} + + def get_token_form_class(self): + return HQAuthenticationTokenForm diff --git a/corehq/apps/hqwebapp/urls.py b/corehq/apps/hqwebapp/urls.py index 6e290e504349..2aa9c85023f5 100644 --- a/corehq/apps/hqwebapp/urls.py +++ b/corehq/apps/hqwebapp/urls.py @@ -79,9 +79,9 @@ url(r'^account/two_factor/backup/tokens/$', TwoFactorBackupTokensView.as_view(), name=TwoFactorBackupTokensView.urlname), url(r'^account/two_factor/disable/$', TwoFactorDisableView.as_view(), name=TwoFactorDisableView.urlname), - url(r'^account/two_factor/backup/phone/register/$', TwoFactorPhoneSetupView.as_view(), + url(r'^account/two_factor/phone/register/$', TwoFactorPhoneSetupView.as_view(), name=TwoFactorPhoneSetupView.urlname), - url(r'^account/two_factor/backup/phone/unregister/(?P\d+)/$', TwoFactorPhoneDeleteView.as_view(), + url(r'^account/two_factor/phone/unregister/(?P\d+)/$', TwoFactorPhoneDeleteView.as_view(), name=TwoFactorPhoneDeleteView.urlname), url(r'', include(tf_urls)), url(r'', include(tf_twilio_urls)), diff --git a/corehq/apps/hqwebapp/utils/two_factor.py b/corehq/apps/hqwebapp/utils/two_factor.py new file mode 100644 index 000000000000..74fd0eddf1e5 --- /dev/null +++ b/corehq/apps/hqwebapp/utils/two_factor.py @@ -0,0 +1,8 @@ +from django.conf import settings + + +def user_can_use_phone(user): + if not settings.ALLOW_PHONE_AS_DEFAULT_TWO_FACTOR_DEVICE: + return False + + return user.belongs_to_messaging_domain() diff --git a/corehq/apps/hqwebapp/views.py b/corehq/apps/hqwebapp/views.py index 1dc284d631c7..21e5ff3ffae2 100644 --- a/corehq/apps/hqwebapp/views.py +++ b/corehq/apps/hqwebapp/views.py @@ -48,6 +48,7 @@ from django.views.generic.base import View from memoized import memoized from sentry_sdk import last_event_id +from two_factor.utils import default_device from two_factor.views import LoginView from corehq.apps.accounting.decorators import ( @@ -478,14 +479,32 @@ def iframe_sso_login_pending(request): class HQLoginView(LoginView): form_list = [ - ('auth', EmailAuthenticationForm), - ('token', HQAuthenticationTokenForm), - ('backup', HQBackupTokenForm), + (LoginView.AUTH_STEP, EmailAuthenticationForm), + (LoginView.TOKEN_STEP, HQAuthenticationTokenForm), + (LoginView.BACKUP_STEP, HQBackupTokenForm), ] extra_context = {} + def has_token_step(self): + """ + Overrides the two_factor LoginView has_token_step to ensure this step is excluded if a valid backup + token exists. Created https://github.com/jazzband/django-two-factor-auth/issues/709 to track work to + potentially include this in django-two-factor-auth directly. + """ + return ( + default_device(self.get_user()) + and self.BACKUP_STEP not in self.storage.validated_step_data + and not self.remember_agent + ) + + # override two_factor LoginView condition_dict to include the method defined above + condition_dict = { + LoginView.TOKEN_STEP: has_token_step, + LoginView.BACKUP_STEP: LoginView.has_backup_step, + } + def post(self, *args, **kwargs): - if settings.ENFORCE_SSO_LOGIN and self.steps.current == 'auth': + if settings.ENFORCE_SSO_LOGIN and self.steps.current == self.AUTH_STEP: # catch anyone who by-passes the javascript and tries to log in directly username = self.request.POST.get('auth-username') idp = IdentityProvider.get_required_identity_provider(username) if username else None @@ -504,7 +523,7 @@ def get_context_data(self, **kwargs): context.update(self.extra_context) context['enforce_sso_login'] = ( settings.ENFORCE_SSO_LOGIN - and self.steps.current == 'auth' + and self.steps.current == self.AUTH_STEP ) domain = context.get('domain') if domain and not is_domain_using_sso(domain): @@ -516,9 +535,9 @@ def get_context_data(self, **kwargs): class CloudCareLoginView(HQLoginView): form_list = [ - ('auth', CloudCareAuthenticationForm), - ('token', HQAuthenticationTokenForm), - ('backup', HQBackupTokenForm), + (HQLoginView.AUTH_STEP, CloudCareAuthenticationForm), + (HQLoginView.TOKEN_STEP, HQAuthenticationTokenForm), + (HQLoginView.BACKUP_STEP, HQBackupTokenForm), ] diff --git a/corehq/apps/settings/forms.py b/corehq/apps/settings/forms.py index ac3c27845daa..d45282bc5bc7 100644 --- a/corehq/apps/settings/forms.py +++ b/corehq/apps/settings/forms.py @@ -15,11 +15,13 @@ from two_factor.forms import ( DeviceValidationForm, MethodForm, + TOTPDeviceForm, +) +from two_factor.plugins.phonenumber.forms import ( PhoneNumberForm, PhoneNumberMethodForm, - TOTPDeviceForm, ) -from two_factor.models import get_available_phone_methods +from two_factor.plugins.phonenumber.utils import get_available_phone_methods from two_factor.utils import totp_digits from corehq.apps.hqwebapp import crispy as hqcrispy @@ -103,8 +105,8 @@ def __init__(self, **kwargs): class HQDeviceValidationForm(DeviceValidationForm): token = forms.IntegerField(required=False, label=_("Token"), min_value=1, max_value=int('9' * totp_digits())) - def __init__(self, **kwargs): - super(HQDeviceValidationForm, self).__init__(**kwargs) + def __init__(self, device, **kwargs): + super(HQDeviceValidationForm, self).__init__(device, **kwargs) self.helper = FormHelper() self.helper.form_class = 'form form-horizontal' self.helper.label_class = 'col-sm-3 col-md-4 col-lg-2' @@ -139,17 +141,17 @@ def clean_token(self): class HQTwoFactorMethodForm(MethodForm): - def __init__(self, *, allow_phone_2fa, **kwargs): super().__init__(**kwargs) if not allow_phone_2fa: # Block people from setting up the phone method as their default - phone_methods = [method for method, _ in get_available_phone_methods()] + phone_methods = [method.code for method in get_available_phone_methods()] self.fields['method'].choices = [ (method, display_name) for method, display_name in self.fields['method'].choices if method not in phone_methods ] + self.helper = FormHelper() self.helper.form_class = 'form form-horizontal' self.helper.label_class = 'col-sm-3 col-md-4 col-lg-2' @@ -170,7 +172,7 @@ def __init__(self, *, allow_phone_2fa, **kwargs): _("Back"), css_class='btn-default', type='submit', - value='welcome_setup', + value='welcome', name="wizard_goto_step", ), ) @@ -180,8 +182,8 @@ def __init__(self, *, allow_phone_2fa, **kwargs): class HQTOTPDeviceForm(TOTPDeviceForm): token = forms.IntegerField(required=False, label=_("Token"), min_value=1, max_value=int('9' * totp_digits())) - def __init__(self, **kwargs): - super(HQTOTPDeviceForm, self).__init__(**kwargs) + def __init__(self, key, user, **kwargs): + super(HQTOTPDeviceForm, self).__init__(key, user, **kwargs) self.helper = FormHelper() self.helper.form_class = 'form form-horizontal' self.helper.label_class = 'col-sm-3 col-md-4 col-lg-2' diff --git a/corehq/apps/settings/tests/test_views.py b/corehq/apps/settings/tests/test_views.py index b5b999d8d321..46780bbaa30f 100644 --- a/corehq/apps/settings/tests/test_views.py +++ b/corehq/apps/settings/tests/test_views.py @@ -2,12 +2,15 @@ from io import BytesIO from unittest.mock import Mock, patch +from django.contrib.sessions.middleware import SessionMiddleware from django.http.response import Http404, HttpResponse from django.test import SimpleTestCase, TestCase, override_settings from django.test.client import RequestFactory from django.urls import reverse -from two_factor.views import PhoneSetupView, ProfileView, SetupView +from django_otp.plugins.otp_totp.models import TOTPDevice +from two_factor.views import ProfileView, SetupView +from two_factor.plugins.phonenumber.views import PhoneSetupView from corehq.apps.domain.shortcuts import create_domain from corehq.apps.users.audit.change_messages import UserChangeMessage @@ -21,6 +24,7 @@ EnableMobilePrivilegesView, TwoFactorPhoneSetupView, TwoFactorProfileView, + TwoFactorResetView, TwoFactorSetupView, get_qrcode, ) @@ -256,3 +260,44 @@ def test_get_qrcode(self): rendered_bytes, f"Rendered PNG differs from expected reference: {reference_fpath}", ) + + +class TestTwoFactorResetView(TestCase): + + def test_default_device_is_deleted(self): + default_device = TOTPDevice.objects.create(user=self.couch_user.get_django_user(), name='default') + self._call_view(self.couch_user) + with self.assertRaises(TOTPDevice.DoesNotExist): + TOTPDevice.objects.get(id=default_device.id) + + def test_backup_device_is_not_deleted(self): + backup_device = TOTPDevice.objects.create(user=self.couch_user.get_django_user(), name='backup') + self._call_view(self.couch_user) + # should not raise error + TOTPDevice.objects.get(id=backup_device.id) + + def _call_view(self, user): + request = self.factory.get('/some_url') + request.user = user.get_django_user() + request.couch_user = user + + def get_response(request): + return {} + + middleware = SessionMiddleware(get_response) + middleware.process_request(request) + request.session.save() + view = TwoFactorResetView.as_view() + return view(request) + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.domain = create_domain('two-factor-domain') + cls.addClassCleanup(cls.domain.delete) + cls.couch_user = WebUser.create(cls.domain.name, 'test@user.com', 'abc123', None, None) + cls.addClassCleanup(cls.couch_user.delete, None, None) + + def setUp(self): + super().setUp() + self.factory = RequestFactory() diff --git a/corehq/apps/settings/views.py b/corehq/apps/settings/views.py index e9f72ca537b2..fad2d6b6f636 100644 --- a/corehq/apps/settings/views.py +++ b/corehq/apps/settings/views.py @@ -19,18 +19,20 @@ import langcodes import qrcode +from django_otp import devices_for_user from memoized import memoized -from two_factor.models import PhoneDevice -from two_factor.utils import backup_phones, default_device +from two_factor.plugins.phonenumber.utils import backup_phones from two_factor.views import ( BackupTokensView, DisableView, - PhoneDeleteView, - PhoneSetupView, ProfileView, SetupCompleteView, SetupView, ) +from two_factor.plugins.phonenumber.views import ( + PhoneDeleteView, + PhoneSetupView +) from dimagi.utils.web import json_response @@ -48,6 +50,7 @@ from corehq.apps.domain.views.base import BaseDomainView from corehq.apps.hqwebapp.decorators import use_jquery_ui from corehq.apps.hqwebapp.utils import sign +from corehq.apps.hqwebapp.utils.two_factor import user_can_use_phone from corehq.apps.hqwebapp.views import ( BaseSectionPageView, CRUDPaginatedViewMixin, @@ -59,9 +62,7 @@ HQDeviceValidationForm, HQEmptyForm, HQPasswordChangeForm, - HQPhoneNumberForm, HQPhoneNumberMethodForm, - HQTOTPDeviceForm, HQTwoFactorMethodForm, ) from corehq.apps.sso.models import IdentityProvider @@ -361,13 +362,6 @@ def post(self, request, *args, **kwargs): return self.get(request, *args, **kwargs) -def _user_can_use_phone(user): - if not settings.ALLOW_PHONE_AS_DEFAULT_TWO_FACTOR_DEVICE: - return False - - return user.belongs_to_messaging_domain() - - class TwoFactorProfileView(BaseMyAccountView, ProfileView): urlname = 'two_factor_settings' template_name = 'two_factor/profile/profile.html' @@ -394,7 +388,7 @@ def get_context_data(self, **kwargs): # Default device means the user has 2FA already enabled has_existing_backup_phones = bool(context.get('backup_phones')) context.update({ - 'allow_phone_2fa': has_existing_backup_phones or _user_can_use_phone(self.request.couch_user), + 'allow_phone_2fa': has_existing_backup_phones or user_can_use_phone(self.request.couch_user), }) return context @@ -406,27 +400,33 @@ class TwoFactorSetupView(BaseMyAccountView, SetupView): page_title = gettext_lazy("Two Factor Authentication Setup") form_list = ( - ('welcome_setup', HQEmptyForm), + ('welcome', HQEmptyForm), ('method', HQTwoFactorMethodForm), - ('generator', HQTOTPDeviceForm), - ('sms', HQPhoneNumberForm), - ('call', HQPhoneNumberForm), - ('validation', HQDeviceValidationForm), + # other forms are registered on startup in corehq.apps.hqwebapp.apps.HqWebAppConfig ) @method_decorator(active_domains_required) @method_decorator(login_required) def dispatch(self, request, *args, **kwargs): - # this is only here to add the login_required decorator + # this is only here to add decorators return super(TwoFactorSetupView, self).dispatch(request, *args, **kwargs) def get_form_kwargs(self, step=None): kwargs = super().get_form_kwargs(step) if step == 'method': - kwargs.setdefault('allow_phone_2fa', _user_can_use_phone(self.request.couch_user)) + kwargs['allow_phone_2fa'] = user_can_use_phone(self.request.couch_user) return kwargs + def get_form_list(self): + # It would be cool if we could specify our custom validation form in the form_list property + # but SetupView.get_form_list hard codes the default validation form for 'sms' and 'call' methods. + # https://github.com/jazzband/django-two-factor-auth/blob/1.15.5/two_factor/views/core.py#L510-L511 + form_list = super().get_form_list() + if {'sms', 'call'} & set(form_list.keys()): + form_list['validation'] = HQDeviceValidationForm + return form_list + class TwoFactorSetupCompleteView(BaseMyAccountView, SetupCompleteView): urlname = 'two_factor_setup_complete' @@ -492,14 +492,14 @@ class TwoFactorPhoneSetupView(BaseMyAccountView, PhoneSetupView): page_title = gettext_lazy("Two Factor Authentication Phone Setup") form_list = ( - ('method', HQPhoneNumberMethodForm), + ('setup', HQPhoneNumberMethodForm), ('validation', HQDeviceValidationForm), ) @method_decorator(login_required) def dispatch(self, request, *args, **kwargs): has_backup_phones = bool(backup_phones(self.request.user)) - if not (has_backup_phones or _user_can_use_phone(request.couch_user)): + if not (has_backup_phones or user_can_use_phone(request.couch_user)): # NOTE: this behavior could be seen as un-intuitive. If a domain is not authorized to use phone/sms, # we are still allowing full functionality if they have an existing backup phone. The primary reason # is so that a user can delete a backup number if needed. The ability to add in a new number is still @@ -516,15 +516,6 @@ def done(self, form_list, **kwargs): messages.add_message(self.request, messages.SUCCESS, _("Phone number added.")) return redirect(reverse(TwoFactorProfileView.urlname)) - def get_device(self, **kwargs): - """ - Uses the data from the setup step and generated key to recreate device, gets the 'method' step - in the form_list. - """ - kwargs = kwargs or {} - kwargs.update(self.storage.validated_step_data.get('method', {})) - return PhoneDevice(key=self.get_key(), **kwargs) - class TwoFactorPhoneDeleteView(BaseMyAccountView, PhoneDeleteView): @@ -541,17 +532,14 @@ def dispatch(self, request, *args, **kwargs): class TwoFactorResetView(TwoFactorSetupView): urlname = 'reset' - form_list = ( - ('welcome_reset', HQEmptyForm), - ('method', HQTwoFactorMethodForm), - ('generator', HQTOTPDeviceForm), - ('sms', HQPhoneNumberForm), - ('call', HQPhoneNumberForm), - ('validation', HQDeviceValidationForm), - ) - def get(self, request, *args, **kwargs): - default_device(request.user).delete() + # avoid using django-two-factor-auth default_devices method because it adds a dynamic attribute + # to the user object that we then have to cleanup before calling super + # https://github.com/jazzband/django-two-factor-auth/blob/1.15.5/two_factor/utils.py#L9-L17 + for device in devices_for_user(request.user): + if device.name == 'default': + device.delete() + break return super(TwoFactorResetView, self).get(request, *args, **kwargs) diff --git a/migrations.lock b/migrations.lock index 006df383e257..0ec339e544d5 100644 --- a/migrations.lock +++ b/migrations.lock @@ -749,6 +749,8 @@ phonelog 0012_server_date_not_null 0013_delete_olddevicereportentry 0014_auto_20170718_2039 +phonenumber + 0001_squashed_0001_initial (10 squashed migrations) pillow_retry 0001_initial 0002_pillowerror_queued @@ -1111,13 +1113,7 @@ translations 0008_remove_couch_id 0009_auto_20200924_1753 two_factor - 0001_initial - 0002_auto_20150110_0810 - 0003_auto_20150817_1733 - 0004_auto_20160205_1827 - 0005_auto_20160224_0450 - 0006_phonedevice_key_default - 0007_auto_20201201_1019 + (no migrations) user_importer 0001_initial userreports diff --git a/requirements/base-requirements.in b/requirements/base-requirements.in index b021f27708d6..b34871061e5f 100644 --- a/requirements/base-requirements.in +++ b/requirements/base-requirements.in @@ -37,7 +37,7 @@ django-recaptcha django-statici18n django-tastypie django-transfer -django-two-factor-auth +django-two-factor-auth @ git+https://github.com/jazzband/django-two-factor-auth.git@16f688bf329526d897a33594ab598bcd3fc8eaae # waiting for next release after 1.16.0 django-user-agents django-websocket-redis @ https://raw.githubusercontent.com/dimagi/django-websocket-redis/0.6.0.1/releases/django_websocket_redis-0.6.0.1-py3-none-any.whl django>=3.2.18,<4.0 diff --git a/requirements/dev-requirements.txt b/requirements/dev-requirements.txt index 354462322a7d..6ccb771621f5 100644 --- a/requirements/dev-requirements.txt +++ b/requirements/dev-requirements.txt @@ -215,7 +215,7 @@ django-tastypie==0.14.6 # via -r base-requirements.in django-transfer==0.4 # via -r base-requirements.in -django-two-factor-auth==1.13.2 +django-two-factor-auth @ git+https://github.com/jazzband/django-two-factor-auth.git@16f688bf329526d897a33594ab598bcd3fc8eaae # via -r base-requirements.in django-user-agents==0.4.0 # via -r base-requirements.in diff --git a/requirements/docs-requirements.txt b/requirements/docs-requirements.txt index 814233bf3cfb..19d5e3e289bc 100644 --- a/requirements/docs-requirements.txt +++ b/requirements/docs-requirements.txt @@ -192,7 +192,7 @@ django-tastypie==0.14.6 # via -r base-requirements.in django-transfer==0.4 # via -r base-requirements.in -django-two-factor-auth==1.13.2 +django-two-factor-auth @ git+https://github.com/jazzband/django-two-factor-auth.git@16f688bf329526d897a33594ab598bcd3fc8eaae # via -r base-requirements.in django-user-agents==0.4.0 # via -r base-requirements.in diff --git a/requirements/prod-requirements.txt b/requirements/prod-requirements.txt index b06d0d5461e4..75d4a2c42fdb 100644 --- a/requirements/prod-requirements.txt +++ b/requirements/prod-requirements.txt @@ -190,7 +190,7 @@ django-tastypie==0.14.6 # via -r base-requirements.in django-transfer==0.4 # via -r base-requirements.in -django-two-factor-auth==1.13.2 +django-two-factor-auth @ git+https://github.com/jazzband/django-two-factor-auth.git@16f688bf329526d897a33594ab598bcd3fc8eaae # via -r base-requirements.in django-user-agents==0.4.0 # via -r base-requirements.in diff --git a/requirements/requirements.txt b/requirements/requirements.txt index aef6a4e94fd5..6db4544352eb 100644 --- a/requirements/requirements.txt +++ b/requirements/requirements.txt @@ -183,7 +183,7 @@ django-tastypie==0.14.6 # via -r base-requirements.in django-transfer==0.4 # via -r base-requirements.in -django-two-factor-auth==1.13.2 +django-two-factor-auth @ git+https://github.com/jazzband/django-two-factor-auth.git@16f688bf329526d897a33594ab598bcd3fc8eaae # via -r base-requirements.in django-user-agents==0.4.0 # via -r base-requirements.in diff --git a/requirements/test-requirements.txt b/requirements/test-requirements.txt index 94ce38ab0957..572ff4f94671 100644 --- a/requirements/test-requirements.txt +++ b/requirements/test-requirements.txt @@ -192,7 +192,7 @@ django-tastypie==0.14.6 # via -r base-requirements.in django-transfer==0.4 # via -r base-requirements.in -django-two-factor-auth==1.13.2 +django-two-factor-auth @ git+https://github.com/jazzband/django-two-factor-auth.git@16f688bf329526d897a33594ab598bcd3fc8eaae # via -r base-requirements.in django-user-agents==0.4.0 # via -r base-requirements.in diff --git a/settings.py b/settings.py index 4da86715c844..1631e96e6b36 100755 --- a/settings.py +++ b/settings.py @@ -231,6 +231,7 @@ 'django_otp.plugins.otp_static', 'django_otp.plugins.otp_totp', 'two_factor', + 'two_factor.plugins.phonenumber', 'ws4redis', 'statici18n', 'django_user_agents', @@ -1093,6 +1094,10 @@ def _pkce_required(client_id): # Disable builtin throttling for two factor backup tokens, since we have our own # See corehq.apps.hqwebapp.signals and corehq.apps.hqwebapp.forms for details OTP_STATIC_THROTTLE_FACTOR = 0 +# Adding OTP_TOTP_THROTTLE_FACTOR and TWO_FACTOR_PHONE_THROTTLE_FACTOR to preserve behavior after upgrading +# past version 1.15.4 of django-two-factor-auth, which changed the factor to 10. +OTP_TOTP_THROTTLE_FACTOR = 1 +TWO_FACTOR_PHONE_THROTTLE_FACTOR = 1 ALLOW_PHONE_AS_DEFAULT_TWO_FACTOR_DEVICE = False RATE_LIMIT_SUBMISSIONS = False