From fca3ac9edb591c94feca87c771b6dfd313646517 Mon Sep 17 00:00:00 2001 From: Guillermo Date: Mon, 27 Oct 2025 15:37:39 -0600 Subject: [PATCH 1/6] Assert it redirects to the allauth mfa auth path --- kobo/apps/accounts/mfa/tests/test_login.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kobo/apps/accounts/mfa/tests/test_login.py b/kobo/apps/accounts/mfa/tests/test_login.py index 9641f78b47..35c7d197f6 100644 --- a/kobo/apps/accounts/mfa/tests/test_login.py +++ b/kobo/apps/accounts/mfa/tests/test_login.py @@ -50,7 +50,7 @@ def test_login_with_mfa_enabled(self): 'password': 'someuser', } response = self.client.post(reverse('kobo_login'), data=data) - self.assertContains(response, 'verification token') + self.assertRedirects(response, reverse('mfa_authenticate')) def test_login_with_mfa_disabled(self): """ From cbd1e163b5b39d9de650e1f6be8466d3b520ed42 Mon Sep 17 00:00:00 2001 From: Guillermo Date: Mon, 27 Oct 2025 16:41:18 -0600 Subject: [PATCH 2/6] Replace trench based form and redirects with allauth mfa features --- kobo/apps/accounts/adapter.py | 42 -------- kobo/apps/accounts/mfa/forms.py | 98 +------------------ .../mfa/templates/mfa/authenticate.html | 57 +++++++++++ .../accounts/mfa/templates/mfa_token.html | 62 ------------ kobo/apps/accounts/mfa/tests/test_login.py | 1 - kobo/apps/accounts/mfa/urls.py | 2 - kobo/apps/accounts/mfa/views.py | 17 +--- kobo/apps/stripe/tests/test_mfa_login.py | 4 +- kobo/settings/base.py | 8 +- 9 files changed, 68 insertions(+), 223 deletions(-) create mode 100644 kobo/apps/accounts/mfa/templates/mfa/authenticate.html delete mode 100644 kobo/apps/accounts/mfa/templates/mfa_token.html diff --git a/kobo/apps/accounts/adapter.py b/kobo/apps/accounts/adapter.py index 7fac2e150d..301bc4f303 100644 --- a/kobo/apps/accounts/adapter.py +++ b/kobo/apps/accounts/adapter.py @@ -8,16 +8,12 @@ from django.template.response import TemplateResponse from django.utils import timezone from trench.utils import get_mfa_model, user_token_generator - -from .mfa.forms import MfaTokenForm from .mfa.models import MfaAvailableToUser from .mfa.permissions import mfa_allowed_for_user -from .mfa.views import MfaTokenView from .utils import user_has_inactive_paid_subscription class AccountAdapter(DefaultAccountAdapter): - def is_open_for_signup(self, request): return config.REGISTRATION_OPEN @@ -26,44 +22,6 @@ def login(self, request, user): user.backend = settings.AUTHENTICATION_BACKENDS[0] super().login(request, user) - def pre_login(self, request, user, **kwargs): - - if parent_response := super().pre_login(request, user, **kwargs): - # A response from the parent means the login process must be - # interrupted, e.g. due to the user being inactive or not having - # validated their email address - return parent_response - - # If MFA is activated and allowed for the user, display the token form before letting them in - mfa_active = ( - get_mfa_model().objects.filter(is_active=True, user=user).exists() - ) - mfa_allowed = mfa_allowed_for_user(user) - inactive_subscription = user_has_inactive_paid_subscription( - user.username - ) - if mfa_active and (mfa_allowed or inactive_subscription): - ephemeral_token_cache = user_token_generator.make_token(user) - mfa_token_form = MfaTokenForm( - initial={'ephemeral_token': ephemeral_token_cache} - ) - - next_url = kwargs.get('redirect_url') or resolve_url( - settings.LOGIN_REDIRECT_URL - ) - - context = { - REDIRECT_FIELD_NAME: next_url, - 'view': MfaTokenView, - 'form': mfa_token_form, - } - - return TemplateResponse( - request=request, - template='mfa_token.html', - context=context, - ) - def save_user(self, request, user, form, commit=True): # Compare allauth SignupForm with our custom field standard_fields = set(SignupForm().fields.keys()) diff --git a/kobo/apps/accounts/mfa/forms.py b/kobo/apps/accounts/mfa/forms.py index 0d0956d816..d3c68bce70 100644 --- a/kobo/apps/accounts/mfa/forms.py +++ b/kobo/apps/accounts/mfa/forms.py @@ -1,4 +1,4 @@ -# coding: utf-8 +from allauth.mfa.base.forms import AuthenticateForm from django import forms from django.conf import settings from django.utils.translation import gettext_lazy as t @@ -9,98 +9,6 @@ from trench.serializers import CodeLoginSerializer from trench.utils import get_mfa_model, user_token_generator -from kobo.apps.accounts.forms import LoginForm - -class MfaLoginForm(LoginForm): - """ - Authenticating users. - If 2FA is activated, first step (of two) of the login process. - """ - - def __init__(self, *args, **kwargs): - self.ephemeral_token_cache = None - super().__init__(*args, **kwargs) - - def clean(self, *args, **kwargs): - cleaned_data = super().clean(*args, **kwargs) - # `super().clean()` initialize the object `self.user` with - # the user object retrieved from authentication (if any) - # Because we only support one 2FA method, we do not filter on - # `is_primary` too (as django_trench does). - # ToDo Figure out why `is_primary` is False sometimes after reactivating - # 2FA - if get_mfa_model().objects.filter(is_active=True, user=self.user).exists(): - self.ephemeral_token_cache = user_token_generator.make_token( - self.user - ) - - return cleaned_data - - def get_ephemeral_token(self): - return self.ephemeral_token_cache - - -class MfaTokenForm(forms.Form): - """ - Validate 2FA token. - Second (and last) step of login process when MFA is activated. - """ - - code = forms.CharField( - label='', - strip=True, - required=True, - widget=forms.TextInput( - attrs={ - 'placeholder': t( - 'Enter the ##token length##-character token' - ).replace( - '##token length##', str(settings.TRENCH_AUTH['CODE_LENGTH']) - ) - } - ), - ) - ephemeral_token = forms.CharField( - required=True, - widget=forms.HiddenInput(), - ) - - error_messages = {'invalid_code': t('Your token is invalid')} - - def __init__(self, request=None, *args, **kwargs): - self.user_cache = None - super().__init__(*args, **kwargs) - - def clean(self): - code_login_serializer = CodeLoginSerializer(data=self.cleaned_data) - if not code_login_serializer.is_valid(): - raise self.get_invalid_mfa_error() - - try: - self.user_cache = authenticate_second_step_command( - code=code_login_serializer.validated_data['code'], - ephemeral_token=code_login_serializer.validated_data[ - 'ephemeral_token' - ], - ) - except MFAValidationError: - raise self.get_invalid_mfa_error() - - # When login is successful, `django.contrib.auth.login()` expects the - # authentication backend class to be attached to user object. - # See https://github.com/django/django/blob/b87820668e7bd519dbc05f6ee46f551858fb1d6d/django/contrib/auth/__init__.py#L111 - # Since we do not have a bullet-proof way to detect which authentication - # class is the good one, we use the first element of the list - self.user_cache.backend = settings.AUTHENTICATION_BACKENDS[0] - - return self.cleaned_data - - def get_invalid_mfa_error(self): - return forms.ValidationError( - self.error_messages['invalid_code'], - code='invalid_code', - ) - - def get_user(self): - return self.user_cache +class MfaAuthenticateForm(AuthenticateForm): + pass diff --git a/kobo/apps/accounts/mfa/templates/mfa/authenticate.html b/kobo/apps/accounts/mfa/templates/mfa/authenticate.html new file mode 100644 index 0000000000..d0c2471a35 --- /dev/null +++ b/kobo/apps/accounts/mfa/templates/mfa/authenticate.html @@ -0,0 +1,57 @@ +{% extends "account/base.html" %} {% load static %} {% load i18n %} {% block content %} + + +{% endblock %} {% block extra_javascript %} + +{% endblock %} diff --git a/kobo/apps/accounts/mfa/templates/mfa_token.html b/kobo/apps/accounts/mfa/templates/mfa_token.html deleted file mode 100644 index bc83364680..0000000000 --- a/kobo/apps/accounts/mfa/templates/mfa_token.html +++ /dev/null @@ -1,62 +0,0 @@ -{% extends "account/base.html" %} -{% load static %} -{% load i18n %} -{% block content %} - - -{% endblock %} -{% block extra_javascript %} - -{% endblock %} diff --git a/kobo/apps/accounts/mfa/tests/test_login.py b/kobo/apps/accounts/mfa/tests/test_login.py index 35c7d197f6..d3b27f02e5 100644 --- a/kobo/apps/accounts/mfa/tests/test_login.py +++ b/kobo/apps/accounts/mfa/tests/test_login.py @@ -39,7 +39,6 @@ def setUp(self): # Ensure `self.client` is not authenticated self.client.logout() - @pytest.mark.skip(reason='MFA Forms not replaced yet...') def test_login_with_mfa_enabled(self): """ Validate that multi-factor authentication form is displayed after diff --git a/kobo/apps/accounts/mfa/urls.py b/kobo/apps/accounts/mfa/urls.py index 540827434f..4723cb6e5e 100644 --- a/kobo/apps/accounts/mfa/urls.py +++ b/kobo/apps/accounts/mfa/urls.py @@ -8,11 +8,9 @@ MfaMethodConfirmView, MfaMethodDeactivateView, MfaMethodRegenerateCodesView, - MfaTokenView, ) urlpatterns = [ - path('accounts/login/mfa/', MfaTokenView.as_view(), name='mfa_token'), path('accounts/login/', MfaLoginView.as_view(), name='kobo_login'), path( 'api/v2/auth/mfa/user-methods/', diff --git a/kobo/apps/accounts/mfa/views.py b/kobo/apps/accounts/mfa/views.py index 82dba50d8a..499d544c9f 100644 --- a/kobo/apps/accounts/mfa/views.py +++ b/kobo/apps/accounts/mfa/views.py @@ -14,14 +14,15 @@ from kpi.permissions import IsAuthenticated from kpi.utils.log import logging from .flows import activate_totp, deactivate_totp, regenerate_codes -from .forms import MfaLoginForm, MfaTokenForm +from .forms import MfaAuthenticateForm from .models import MfaMethodsWrapper from .permissions import IsMfaEnabled from .serializers import TOTPCodeSerializer, UserMfaMethodSerializer +from ..forms import LoginForm class MfaLoginView(LoginView): - form_class = MfaLoginForm + form_class = LoginForm def get_success_url(self): """ @@ -51,18 +52,6 @@ def get_success_url(self): return redirect_to -class MfaTokenView(DjangoLoginView): - """ - Display the login form and handle the login action. - """ - - form_class = MfaTokenForm - authentication_form = None - template_name = 'mfa_token.html' - redirect_authenticated_user = False - extra_context = None - - class MfaListUserMethodsView(ListAPIView): """ Display user's methods with dates diff --git a/kobo/apps/stripe/tests/test_mfa_login.py b/kobo/apps/stripe/tests/test_mfa_login.py index b50f2d24e8..6a1008a1f8 100644 --- a/kobo/apps/stripe/tests/test_mfa_login.py +++ b/kobo/apps/stripe/tests/test_mfa_login.py @@ -12,7 +12,7 @@ from trench.utils import get_mfa_model from kobo.apps.kobo_auth.shortcuts import User -from kobo.apps.accounts.mfa.forms import MfaLoginForm +from kobo.apps.accounts.forms import LoginForm from kobo.apps.accounts.mfa.models import MfaAvailableToUser from kobo.apps.organizations.models import Organization, OrganizationUser from kpi.tests.kpi_test_case import KpiTestCase @@ -254,7 +254,7 @@ def test_no_mfa_login_with_wrong_password(self): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertIsInstance(response, TemplateResponse) self.assertFalse(response.context_data['form'].is_valid()) - self.assertIsInstance(response.context_data['form'], MfaLoginForm) + self.assertIsInstance(response.context_data['form'], LoginForm) @override_config(MFA_ENABLED=True) def test_mfa_login_per_user_availability_no_subscription(self): diff --git a/kobo/settings/base.py b/kobo/settings/base.py index 12e6213cbe..0b238ffd4b 100644 --- a/kobo/settings/base.py +++ b/kobo/settings/base.py @@ -99,6 +99,7 @@ 'private_storage', 'kobo.apps.KpiConfig', 'kobo.apps.accounts', + 'kobo.apps.accounts.mfa.apps.MfaAppConfig', 'allauth', 'allauth.account', 'allauth.socialaccount', @@ -130,7 +131,6 @@ 'markdownx', 'kobo.apps.help', 'trench', - 'kobo.apps.accounts.mfa.apps.MfaAppConfig', 'kobo.apps.project_views.apps.ProjectViewAppConfig', 'kobo.apps.languages.apps.LanguageAppConfig', 'kobo.apps.audit_log.AuditLogAppConfig', @@ -1544,7 +1544,7 @@ def dj_stripe_request_callback_method(): ACCOUNT_SIGNUP_FIELDS = ['email*', 'username*', 'password1*', 'password2*'] ACCOUNT_EMAIL_VERIFICATION = env.str('ACCOUNT_EMAIL_VERIFICATION', 'mandatory') ACCOUNT_FORMS = { - 'login': 'kobo.apps.accounts.mfa.forms.MfaLoginForm', + 'login': 'kobo.apps.accounts.forms.LoginForm', 'signup': 'kobo.apps.accounts.forms.SignupForm', } ACCOUNT_LOGIN_ON_EMAIL_CONFIRMATION = True @@ -1917,9 +1917,7 @@ def dj_stripe_request_callback_method(): KOBOCAT_MEDIA_URL = f'{KOBOCAT_URL}/media/' -MFA_FORMS = { - 'authenticate': 'kobo.apps.accounts.mfa.forms.MfaTokenForm', -} +MFA_FORMS = {} MFA_ADAPTER = 'kobo.apps.accounts.mfa.adapter.MfaAdapter' MFA_TOTP_DIGITS = env.int('MFA_CODE_LENGTH', 6) MFA_RECOVERY_CODE_COUNT = 5 From d1aa066d501928e4794d673beb4ed66f1eac14c9 Mon Sep 17 00:00:00 2001 From: Guillermo Date: Mon, 27 Oct 2025 16:46:48 -0600 Subject: [PATCH 3/6] Format python --- kobo/apps/accounts/adapter.py | 7 --- kobo/apps/accounts/mfa/forms.py | 9 ---- kobo/apps/accounts/mfa/tests/test_api.py | 4 +- kobo/apps/accounts/mfa/tests/test_login.py | 1 - kobo/apps/accounts/mfa/views.py | 4 +- .../openrosa/apps/main/tests/test_auth.py | 2 +- kobo/apps/stripe/tests/test_mfa_login.py | 53 +++++-------------- kpi/tests/api/v2/test_api_authentication.py | 4 +- 8 files changed, 19 insertions(+), 65 deletions(-) diff --git a/kobo/apps/accounts/adapter.py b/kobo/apps/accounts/adapter.py index 301bc4f303..f18bf15e74 100644 --- a/kobo/apps/accounts/adapter.py +++ b/kobo/apps/accounts/adapter.py @@ -2,15 +2,8 @@ from allauth.account.forms import SignupForm from constance import config from django.conf import settings -from django.contrib.auth import REDIRECT_FIELD_NAME, login from django.db import transaction -from django.shortcuts import resolve_url -from django.template.response import TemplateResponse from django.utils import timezone -from trench.utils import get_mfa_model, user_token_generator -from .mfa.models import MfaAvailableToUser -from .mfa.permissions import mfa_allowed_for_user -from .utils import user_has_inactive_paid_subscription class AccountAdapter(DefaultAccountAdapter): diff --git a/kobo/apps/accounts/mfa/forms.py b/kobo/apps/accounts/mfa/forms.py index d3c68bce70..7267395125 100644 --- a/kobo/apps/accounts/mfa/forms.py +++ b/kobo/apps/accounts/mfa/forms.py @@ -1,13 +1,4 @@ from allauth.mfa.base.forms import AuthenticateForm -from django import forms -from django.conf import settings -from django.utils.translation import gettext_lazy as t -from trench.command.authenticate_second_factor import ( - authenticate_second_step_command, -) -from trench.exceptions import MFAValidationError -from trench.serializers import CodeLoginSerializer -from trench.utils import get_mfa_model, user_token_generator class MfaAuthenticateForm(AuthenticateForm): diff --git a/kobo/apps/accounts/mfa/tests/test_api.py b/kobo/apps/accounts/mfa/tests/test_api.py index 0779b4f843..77328644a9 100644 --- a/kobo/apps/accounts/mfa/tests/test_api.py +++ b/kobo/apps/accounts/mfa/tests/test_api.py @@ -23,9 +23,7 @@ def setUp(self): # Activate MFA for someuser self.client.login(username='someuser', password='someuser') - self.client.post( - reverse('mfa-activate', kwargs={'method': METHOD}) - ) + self.client.post(reverse('mfa-activate', kwargs={'method': METHOD})) code = get_mfa_code_for_user(self.someuser) self.client.post( reverse('mfa-confirm', kwargs={'method': METHOD}), data={'code': str(code)} diff --git a/kobo/apps/accounts/mfa/tests/test_login.py b/kobo/apps/accounts/mfa/tests/test_login.py index d3b27f02e5..968926186b 100644 --- a/kobo/apps/accounts/mfa/tests/test_login.py +++ b/kobo/apps/accounts/mfa/tests/test_login.py @@ -1,4 +1,3 @@ -import pytest from allauth.account.models import EmailAddress from django.conf import settings from django.shortcuts import resolve_url diff --git a/kobo/apps/accounts/mfa/views.py b/kobo/apps/accounts/mfa/views.py index 499d544c9f..510cd96dfe 100644 --- a/kobo/apps/accounts/mfa/views.py +++ b/kobo/apps/accounts/mfa/views.py @@ -2,7 +2,6 @@ from allauth.mfa.adapter import get_adapter from allauth.mfa.internal.flows.add import validate_can_add_authenticator from allauth.mfa.totp.internal import auth as totp_auth -from django.contrib.auth.views import LoginView as DjangoLoginView from django.db.models import QuerySet from django.urls import reverse from rest_framework.generics import ListAPIView @@ -13,12 +12,11 @@ from kpi.permissions import IsAuthenticated from kpi.utils.log import logging +from ..forms import LoginForm from .flows import activate_totp, deactivate_totp, regenerate_codes -from .forms import MfaAuthenticateForm from .models import MfaMethodsWrapper from .permissions import IsMfaEnabled from .serializers import TOTPCodeSerializer, UserMfaMethodSerializer -from ..forms import LoginForm class MfaLoginView(LoginView): diff --git a/kobo/apps/openrosa/apps/main/tests/test_auth.py b/kobo/apps/openrosa/apps/main/tests/test_auth.py index e5004c8b9a..eb76db3ae4 100644 --- a/kobo/apps/openrosa/apps/main/tests/test_auth.py +++ b/kobo/apps/openrosa/apps/main/tests/test_auth.py @@ -4,8 +4,8 @@ from django.urls import reverse from rest_framework import status -from kobo.apps.openrosa.apps.main.tests.test_base import TestBase from kobo.apps.accounts.mfa.tests.utils import get_mfa_code_for_user +from kobo.apps.openrosa.apps.main.tests.test_base import TestBase class TestAuthBase(TestBase): diff --git a/kobo/apps/stripe/tests/test_mfa_login.py b/kobo/apps/stripe/tests/test_mfa_login.py index 6a1008a1f8..bc3de1420e 100644 --- a/kobo/apps/stripe/tests/test_mfa_login.py +++ b/kobo/apps/stripe/tests/test_mfa_login.py @@ -4,16 +4,15 @@ from django.conf import settings from django.shortcuts import resolve_url from django.template.response import TemplateResponse -from django.test import override_settings from django.urls import reverse -from djstripe.models import Customer, Price, SubscriptionItem, Subscription +from djstripe.models import Customer, Price, Subscription, SubscriptionItem from model_bakery import baker from rest_framework import status from trench.utils import get_mfa_model -from kobo.apps.kobo_auth.shortcuts import User from kobo.apps.accounts.forms import LoginForm from kobo.apps.accounts.mfa.models import MfaAvailableToUser +from kobo.apps.kobo_auth.shortcuts import User from kobo.apps.organizations.models import Organization, OrganizationUser from kpi.tests.kpi_test_case import KpiTestCase @@ -24,9 +23,7 @@ def setUp(self): self.anotheruser = User.objects.get(username='anotheruser') # Confirm someuser's e-mail address as primary and verified - email_address, _ = EmailAddress.objects.get_or_create( - user=self.someuser - ) + email_address, _ = EmailAddress.objects.get_or_create(user=self.someuser) email_address.primary = True email_address.verified = True email_address.save() @@ -43,9 +40,7 @@ def setUp(self): # Ensure `self.client` is not authenticated self.client.logout() - self.organization = baker.make( - Organization, id='orgSALFMLFMSDGmgdlsgmsd' - ) + self.organization = baker.make(Organization, id='orgSALFMLFMSDGmgdlsgmsd') self.organization_user = baker.make( OrganizationUser, user=self.someuser, @@ -90,9 +85,7 @@ def test_no_mfa_login_without_subscription(self): 'login': 'someuser', 'password': 'someuser', } - response = self.client.post( - reverse('kobo_login'), data=data, follow=True - ) + response = self.client.post(reverse('kobo_login'), data=data, follow=True) self._assert_no_mfa_login(response) @override_config(MFA_ENABLED=True) @@ -123,9 +116,7 @@ def test_mfa_login_without_subscription_no_whitelist(self): 'login': 'someuser', 'password': 'someuser', } - response = self.client.post( - reverse('kobo_login'), data=data, follow=True - ) + response = self.client.post(reverse('kobo_login'), data=data, follow=True) self._assert_mfa_login(response) @override_config(MFA_ENABLED=True) @@ -139,9 +130,7 @@ def test_mfa_login_without_subscription_but_whitelisted(self): 'login': 'someuser', 'password': 'someuser', } - response = self.client.post( - reverse('kobo_login'), data=data, follow=True - ) + response = self.client.post(reverse('kobo_login'), data=data, follow=True) self._assert_mfa_login(response) @override_config(MFA_ENABLED=True) @@ -157,9 +146,7 @@ def test_no_mfa_login_with_free_subscription(self): 'login': 'someuser', 'password': 'someuser', } - response = self.client.post( - reverse('kobo_login'), data=data, follow=True - ) + response = self.client.post(reverse('kobo_login'), data=data, follow=True) self._assert_no_mfa_login(response) @override_config(MFA_ENABLED=True) @@ -175,9 +162,7 @@ def test_mfa_login_with_free_subscription_and_whitelisted(self): 'login': 'someuser', 'password': 'someuser', } - response = self.client.post( - reverse('kobo_login'), data=data, follow=True - ) + response = self.client.post(reverse('kobo_login'), data=data, follow=True) self._assert_mfa_login(response) @override_config(MFA_ENABLED=True) @@ -193,9 +178,7 @@ def test_mfa_login_with_free_subscription_no_whitelist(self): 'login': 'someuser', 'password': 'someuser', } - response = self.client.post( - reverse('kobo_login'), data=data, follow=True - ) + response = self.client.post(reverse('kobo_login'), data=data, follow=True) self._assert_mfa_login(response) @override_config(MFA_ENABLED=True) @@ -212,9 +195,7 @@ def test_mfa_login_with_canceled_subscription_but_previously_set(self): 'login': 'someuser', 'password': 'someuser', } - response = self.client.post( - reverse('kobo_login'), data=data, follow=True - ) + response = self.client.post(reverse('kobo_login'), data=data, follow=True) self._assert_mfa_login(response) @override_config(MFA_ENABLED=True) @@ -231,9 +212,7 @@ def test_no_mfa_login_with_canceled_subscription(self): 'login': 'someuser', 'password': 'someuser', } - response = self.client.post( - reverse('kobo_login'), data=data, follow=True - ) + response = self.client.post(reverse('kobo_login'), data=data, follow=True) self._assert_no_mfa_login(response) @override_config(MFA_ENABLED=True) @@ -247,9 +226,7 @@ def test_no_mfa_login_with_wrong_password(self): 'login': 'someuser', 'password': 'badpassword', } - response = self.client.post( - reverse('kobo_login'), data=data, follow=True - ) + response = self.client.post(reverse('kobo_login'), data=data, follow=True) self.assertEqual(len(response.redirect_chain), 0) self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertIsInstance(response, TemplateResponse) @@ -286,9 +263,7 @@ def test_mfa_globally_disabled_as_user_with_paid_subscription(self): 'login': 'someuser', 'password': 'someuser', } - response = self.client.post( - reverse('kobo_login'), data=data, follow=True - ) + response = self.client.post(reverse('kobo_login'), data=data, follow=True) self.assertEqual(len(response.redirect_chain), 1) redirection, status_code = response.redirect_chain[0] self.assertEqual(status_code, status.HTTP_302_FOUND) diff --git a/kpi/tests/api/v2/test_api_authentication.py b/kpi/tests/api/v2/test_api_authentication.py index 63d348ac53..cc4287b8a8 100644 --- a/kpi/tests/api/v2/test_api_authentication.py +++ b/kpi/tests/api/v2/test_api_authentication.py @@ -1,15 +1,15 @@ # coding: utf-8 import base64 -from django.urls import reverse from django.test import override_settings +from django.urls import reverse from rest_framework import status from rest_framework.authtoken.models import Token +from kobo.apps.accounts.mfa.tests.utils import get_mfa_code_for_user from kobo.apps.kobo_auth.shortcuts import User from kpi.tests.base_test_case import BaseAssetTestCase from kpi.urls.router_api_v2 import URL_NAMESPACE as ROUTER_URL_NAMESPACE -from kobo.apps.accounts.mfa.tests.utils import get_mfa_code_for_user class AuthenticationApiTests(BaseAssetTestCase): From b66b0b91ee300239bd388c27e864604d6ebecb4e Mon Sep 17 00:00:00 2001 From: Guillermo Date: Mon, 27 Oct 2025 22:51:06 -0600 Subject: [PATCH 4/6] Fix mfa stripe tests and improve utils, bring back stripe mfa logic --- kobo/apps/accounts/mfa/adapter.py | 12 ++++++++++-- kobo/apps/accounts/mfa/tests/test_api.py | 12 +++++------- kobo/apps/accounts/mfa/tests/test_login.py | 10 ++-------- kobo/apps/accounts/mfa/tests/utils.py | 11 +++++++++++ kobo/apps/stripe/tests/test_mfa_login.py | 22 ++++++++-------------- 5 files changed, 36 insertions(+), 31 deletions(-) diff --git a/kobo/apps/accounts/mfa/adapter.py b/kobo/apps/accounts/mfa/adapter.py index 84cadd0fe0..f6d44955cf 100644 --- a/kobo/apps/accounts/mfa/adapter.py +++ b/kobo/apps/accounts/mfa/adapter.py @@ -2,13 +2,21 @@ from constance import config from .permissions import mfa_allowed_for_user +from .models import MfaMethodsWrapper +from ..utils import user_has_inactive_paid_subscription class MfaAdapter(DefaultMFAAdapter): def is_mfa_enabled(self, user, types=None) -> bool: - super_enabled = super().is_mfa_enabled(user, types) - return super_enabled and mfa_allowed_for_user(user) + mfa_active_super = super().is_mfa_enabled(user, types) + mfa_active = mfa_active_super and MfaMethodsWrapper.objects.filter( + user=user, is_active=True).first() is not None + mfa_allowed = mfa_allowed_for_user(user) + inactive_subscription = user_has_inactive_paid_subscription( + user.username + ) + return mfa_active and (mfa_allowed or inactive_subscription) def get_totp_label(self, user) -> str: """Returns the label used for representing the given user in a TOTP QR diff --git a/kobo/apps/accounts/mfa/tests/test_api.py b/kobo/apps/accounts/mfa/tests/test_api.py index 77328644a9..c5b06a75ad 100644 --- a/kobo/apps/accounts/mfa/tests/test_api.py +++ b/kobo/apps/accounts/mfa/tests/test_api.py @@ -5,7 +5,7 @@ from kobo.apps.kobo_auth.shortcuts import User from kpi.tests.kpi_test_case import BaseTestCase from ..models import MfaAvailableToUser, MfaMethodsWrapper -from .utils import get_mfa_code_for_user +from .utils import get_mfa_code_for_user, activate_mfa_for_user METHOD = 'app' @@ -22,12 +22,10 @@ def setUp(self): self.someuser = User.objects.get(username='someuser') # Activate MFA for someuser - self.client.login(username='someuser', password='someuser') - self.client.post(reverse('mfa-activate', kwargs={'method': METHOD})) - code = get_mfa_code_for_user(self.someuser) - self.client.post( - reverse('mfa-confirm', kwargs={'method': METHOD}), data={'code': str(code)} - ) + activate_mfa_for_user(self.client, self.someuser) + + # Log in + self.client.force_login(self.someuser) def test_user_methods_with_date(self): diff --git a/kobo/apps/accounts/mfa/tests/test_login.py b/kobo/apps/accounts/mfa/tests/test_login.py index 968926186b..a533746163 100644 --- a/kobo/apps/accounts/mfa/tests/test_login.py +++ b/kobo/apps/accounts/mfa/tests/test_login.py @@ -6,7 +6,7 @@ from kobo.apps.kobo_auth.shortcuts import User from kpi.tests.kpi_test_case import KpiTestCase -from .utils import get_mfa_code_for_user +from .utils import get_mfa_code_for_user, activate_mfa_for_user METHOD = 'app' @@ -28,13 +28,7 @@ def setUp(self): email_address.save() # Activate MFA for someuser - self.client.login(username='someuser', password='someuser') - self.client.post(reverse('mfa-activate', kwargs={'method': 'app'})) - self.client.post(reverse('mfa-activate', kwargs={'method': METHOD})) - code = get_mfa_code_for_user(self.someuser) - self.client.post( - reverse('mfa-confirm', kwargs={'method': METHOD}), data={'code': str(code)} - ) + activate_mfa_for_user(self.client, self.someuser) # Ensure `self.client` is not authenticated self.client.logout() diff --git a/kobo/apps/accounts/mfa/tests/utils.py b/kobo/apps/accounts/mfa/tests/utils.py index 00748c9b69..d690a383b1 100644 --- a/kobo/apps/accounts/mfa/tests/utils.py +++ b/kobo/apps/accounts/mfa/tests/utils.py @@ -1,5 +1,6 @@ import pyotp from allauth.mfa.adapter import get_adapter +from django.urls import reverse from ..models import MfaMethodsWrapper @@ -11,3 +12,13 @@ def get_mfa_code_for_user(user): totp = pyotp.TOTP(secret) code = totp.now() return code + + +def activate_mfa_for_user(client, user): + client.force_login(user) + client.post(reverse('mfa-activate', kwargs={'method': 'app'})) + code = get_mfa_code_for_user(user) + client.post( + reverse('mfa-confirm', kwargs={'method': 'app'}), data={'code': str(code)} + ) + client.logout() diff --git a/kobo/apps/stripe/tests/test_mfa_login.py b/kobo/apps/stripe/tests/test_mfa_login.py index bc3de1420e..1e5da354b7 100644 --- a/kobo/apps/stripe/tests/test_mfa_login.py +++ b/kobo/apps/stripe/tests/test_mfa_login.py @@ -8,10 +8,13 @@ from djstripe.models import Customer, Price, Subscription, SubscriptionItem from model_bakery import baker from rest_framework import status -from trench.utils import get_mfa_model from kobo.apps.accounts.forms import LoginForm from kobo.apps.accounts.mfa.models import MfaAvailableToUser +from kobo.apps.accounts.mfa.tests.utils import ( + get_mfa_code_for_user, + activate_mfa_for_user, +) from kobo.apps.kobo_auth.shortcuts import User from kobo.apps.organizations.models import Organization, OrganizationUser from kpi.tests.kpi_test_case import KpiTestCase @@ -29,14 +32,7 @@ def setUp(self): email_address.save() # Activate MFA for someuser - self.mfa_object = get_mfa_model().objects.create( - user=self.someuser, - secret='dummy_mfa_secret', - name='app', - is_primary=True, - is_active=True, - _backup_codes='dummy_encoded_codes', - ) + activate_mfa_for_user(self.client, self.someuser) # Ensure `self.client` is not authenticated self.client.logout() @@ -64,8 +60,7 @@ def _reset_whitelist(self, whitelisted_users=[]): MfaAvailableToUser.objects.create(user=user) def _assert_mfa_login(self, response): - self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertContains(response, 'verification token') + self.assertRedirects(response, reverse('mfa_authenticate')) def _assert_no_mfa_login(self, response): self.assertEqual(len(response.redirect_chain), 1) @@ -207,7 +202,7 @@ def test_no_mfa_login_with_canceled_subscription(self): """ self._reset_whitelist([self.anotheruser]) self._create_subscription(billing_status='canceled') - self.mfa_object.delete() + data = { 'login': 'someuser', 'password': 'someuser', @@ -247,8 +242,7 @@ def test_mfa_login_per_user_availability_no_subscription(self): 'password': 'someuser', } response = self.client.post(reverse('kobo_login'), data=data) - self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertContains(response, 'verification token') + self._assert_mfa_login(response) @override_config(MFA_ENABLED=False) def test_mfa_globally_disabled_as_user_with_paid_subscription(self): From 758df8ddd19b23f2471bc566a875c69267d2259f Mon Sep 17 00:00:00 2001 From: Guillermo Date: Mon, 27 Oct 2025 22:51:47 -0600 Subject: [PATCH 5/6] Format python --- kobo/apps/accounts/mfa/adapter.py | 15 ++++++++------- kobo/apps/accounts/mfa/tests/test_api.py | 2 +- kobo/apps/accounts/mfa/tests/test_login.py | 2 +- kobo/apps/stripe/tests/test_mfa_login.py | 5 +---- 4 files changed, 11 insertions(+), 13 deletions(-) diff --git a/kobo/apps/accounts/mfa/adapter.py b/kobo/apps/accounts/mfa/adapter.py index f6d44955cf..309a6fd277 100644 --- a/kobo/apps/accounts/mfa/adapter.py +++ b/kobo/apps/accounts/mfa/adapter.py @@ -1,21 +1,22 @@ from allauth.mfa.adapter import DefaultMFAAdapter from constance import config -from .permissions import mfa_allowed_for_user -from .models import MfaMethodsWrapper from ..utils import user_has_inactive_paid_subscription +from .models import MfaMethodsWrapper +from .permissions import mfa_allowed_for_user class MfaAdapter(DefaultMFAAdapter): def is_mfa_enabled(self, user, types=None) -> bool: mfa_active_super = super().is_mfa_enabled(user, types) - mfa_active = mfa_active_super and MfaMethodsWrapper.objects.filter( - user=user, is_active=True).first() is not None - mfa_allowed = mfa_allowed_for_user(user) - inactive_subscription = user_has_inactive_paid_subscription( - user.username + mfa_active = ( + mfa_active_super + and MfaMethodsWrapper.objects.filter(user=user, is_active=True).first() + is not None ) + mfa_allowed = mfa_allowed_for_user(user) + inactive_subscription = user_has_inactive_paid_subscription(user.username) return mfa_active and (mfa_allowed or inactive_subscription) def get_totp_label(self, user) -> str: diff --git a/kobo/apps/accounts/mfa/tests/test_api.py b/kobo/apps/accounts/mfa/tests/test_api.py index c5b06a75ad..ff20f0eb04 100644 --- a/kobo/apps/accounts/mfa/tests/test_api.py +++ b/kobo/apps/accounts/mfa/tests/test_api.py @@ -5,7 +5,7 @@ from kobo.apps.kobo_auth.shortcuts import User from kpi.tests.kpi_test_case import BaseTestCase from ..models import MfaAvailableToUser, MfaMethodsWrapper -from .utils import get_mfa_code_for_user, activate_mfa_for_user +from .utils import activate_mfa_for_user, get_mfa_code_for_user METHOD = 'app' diff --git a/kobo/apps/accounts/mfa/tests/test_login.py b/kobo/apps/accounts/mfa/tests/test_login.py index a533746163..560f321d84 100644 --- a/kobo/apps/accounts/mfa/tests/test_login.py +++ b/kobo/apps/accounts/mfa/tests/test_login.py @@ -6,7 +6,7 @@ from kobo.apps.kobo_auth.shortcuts import User from kpi.tests.kpi_test_case import KpiTestCase -from .utils import get_mfa_code_for_user, activate_mfa_for_user +from .utils import activate_mfa_for_user METHOD = 'app' diff --git a/kobo/apps/stripe/tests/test_mfa_login.py b/kobo/apps/stripe/tests/test_mfa_login.py index 1e5da354b7..60a6783cff 100644 --- a/kobo/apps/stripe/tests/test_mfa_login.py +++ b/kobo/apps/stripe/tests/test_mfa_login.py @@ -11,10 +11,7 @@ from kobo.apps.accounts.forms import LoginForm from kobo.apps.accounts.mfa.models import MfaAvailableToUser -from kobo.apps.accounts.mfa.tests.utils import ( - get_mfa_code_for_user, - activate_mfa_for_user, -) +from kobo.apps.accounts.mfa.tests.utils import activate_mfa_for_user from kobo.apps.kobo_auth.shortcuts import User from kobo.apps.organizations.models import Organization, OrganizationUser from kpi.tests.kpi_test_case import KpiTestCase From 93a5d8fa335252ec7709f325a2f0c24fcbff6851 Mon Sep 17 00:00:00 2001 From: Guillermo Date: Thu, 30 Oct 2025 09:13:13 -0600 Subject: [PATCH 6/6] Fix audit log test related to mfa login --- kobo/apps/audit_log/tests/test_signals.py | 33 ++++++++--------------- 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/kobo/apps/audit_log/tests/test_signals.py b/kobo/apps/audit_log/tests/test_signals.py index 7318369d49..8d431c81cd 100644 --- a/kobo/apps/audit_log/tests/test_signals.py +++ b/kobo/apps/audit_log/tests/test_signals.py @@ -6,8 +6,11 @@ from django.contrib.auth.signals import user_logged_in from django.test import RequestFactory, override_settings from django.urls import reverse -from trench.utils import get_mfa_model +from kobo.apps.accounts.mfa.tests.utils import ( + activate_mfa_for_user, + get_mfa_code_for_user, +) from kobo.apps.audit_log.audit_actions import AuditAction from kobo.apps.audit_log.models import AuditLog from kobo.apps.audit_log.signals import create_access_log @@ -101,18 +104,11 @@ def test_login_with_email_verification(self): self.assertEqual(audit_log.action, AuditAction.AUTH) def test_mfa_login(self): - mfa_object = get_mfa_model().objects.create( - user=AccessLogsSignalsTestCase.user, - secret='dummy_mfa_secret', - name='app', - is_primary=True, - is_active=True, - _backup_codes='dummy_encoded_codes', - ) - mfa_object.save() - email_address, _ = EmailAddress.objects.get_or_create( - user=AccessLogsSignalsTestCase.user - ) + user = AccessLogsSignalsTestCase.user + activate_mfa_for_user(self.client, user) + # Delete audit logs up to this point to start from scratch + AuditLog.objects.all().delete() + email_address, _ = EmailAddress.objects.get_or_create(user=user) email_address.primary = True email_address.verified = True email_address.save() @@ -124,15 +120,8 @@ def test_mfa_login(self): # no audit log should be created yet because the MFA code hasn't been entered self.assertEqual(AuditLog.objects.count(), 0) - with patch( - 'kobo.apps.accounts.mfa.forms.authenticate_second_step_command', - return_value=AccessLogsSignalsTestCase.user, - ): - self.client.post( - reverse('mfa_token'), - data={'code': '123456', 'ephemeral_token': 'dummy'}, - follow=True, - ) + code = get_mfa_code_for_user(user) + self.client.post(reverse('mfa_authenticate'), {'code': code}) self.assertEqual(AuditLog.objects.count(), 1) audit_log = AuditLog.objects.first() self.assertEqual(audit_log.user.id, AccessLogsSignalsTestCase.user.id)