Skip to content

Commit 8228dde

Browse files
authored
feat(mfa): replace trench based API endpoints, views and forms with allauth implementation DEV-749 (#6402)
### Summary Migrate MFA features from Trench to Django Allauth for better integration, maintainability. ### Description This update replaces all Trench-based multi-factor authentication (MFA) endpoints, views, and forms with a new implementation powered by Django Allauth. The change ensures tighter integration with the existing authentication system, simplifies configuration, and improves long-term maintainability.
1 parent 4d44be1 commit 8228dde

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

49 files changed

+1121
-634
lines changed

dependencies/pip/dev_requirements.txt

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ cron-descriptor==1.4.3
115115
cryptography==42.0.5
116116
# via
117117
# azure-storage-blob
118+
# fido2
118119
# jwcrypto
119120
# paramiko
120121
# pyopenssl
@@ -179,7 +180,7 @@ django==4.2.24
179180
# drf-spectacular
180181
# jsonfield
181182
# model-bakery
182-
django-allauth==65.11.2
183+
django-allauth[mfa]==65.11.2
183184
# via -r dependencies/pip/requirements.in
184185
django-amazon-ses==4.0.1
185186
# via -r dependencies/pip/requirements.in
@@ -283,6 +284,8 @@ fabric==3.2.2
283284
# via -r dependencies/pip/dev_requirements.in
284285
fakeredis==2.30.1
285286
# via -r dependencies/pip/dev_requirements.in
287+
fido2==2.0.0
288+
# via django-allauth
286289
flake8==7.1.1
287290
# via
288291
# -r dependencies/pip/dev_requirements.in
@@ -564,6 +567,8 @@ pyyaml==6.0.1
564567
# via
565568
# drf-spectacular
566569
# responses
570+
qrcode==8.2
571+
# via django-allauth
567572
redis==5.0.3
568573
# via
569574
# celery

dependencies/pip/requirements.in

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@ dict2xml
2828
defusedxml
2929
dj-static
3030
dj-stripe
31-
django-allauth
31+
django-allauth>=65.11.2
32+
django-allauth[mfa]
3233
django-braces
3334
django-celery-beat
3435
django-constance
@@ -116,4 +117,4 @@ djangorestframework-jsonp
116117
pandas
117118

118119
# Api Documentation
119-
drf-spectacular
120+
drf-spectacular

dependencies/pip/requirements.txt

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ cron-descriptor==1.4.3
9999
cryptography==42.0.5
100100
# via
101101
# azure-storage-blob
102+
# fido2
102103
# jwcrypto
103104
# pyopenssl
104105
cssselect==1.2.0
@@ -147,7 +148,7 @@ django==4.2.24
147148
# djangorestframework
148149
# drf-spectacular
149150
# jsonfield
150-
django-allauth==65.11.2
151+
django-allauth[mfa]==65.11.2
151152
# via -r dependencies/pip/requirements.in
152153
django-amazon-ses==4.0.1
153154
# via -r dependencies/pip/requirements.in
@@ -237,6 +238,8 @@ drf-spectacular==0.28.0
237238
# via -r dependencies/pip/requirements.in
238239
et-xmlfile==1.1.0
239240
# via openpyxl
241+
fido2==2.0.0
242+
# via django-allauth
240243
flower==2.0.1
241244
# via -r dependencies/pip/requirements.in
242245
frozenlist==1.4.1
@@ -437,6 +440,8 @@ pyyaml==6.0.1
437440
# via
438441
# drf-spectacular
439442
# responses
443+
qrcode==8.2
444+
# via django-allauth
440445
redis==5.0.3
441446
# via
442447
# celery

hub/admin/extend_user.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
from django.utils import timezone
1515
from django.utils.safestring import mark_safe
1616

17-
from kobo.apps.accounts.mfa.models import MfaMethod
17+
from kobo.apps.accounts.mfa.models import MfaMethodsWrapper
1818
from kobo.apps.accounts.validators import (
1919
USERNAME_INVALID_MESSAGE,
2020
USERNAME_MAX_LENGTH,
@@ -27,7 +27,6 @@
2727
from kobo.apps.trash_bin.models.account import AccountTrash
2828
from kobo.apps.trash_bin.utils import move_to_trash
2929
from kpi.models.asset import AssetDeploymentStatus
30-
3130
from .filters import UserAdvancedSearchFilter
3231
from .mixins import AdvancedSearchMixin
3332

@@ -37,7 +36,7 @@ def validate_superuser_auth(obj) -> bool:
3736
obj.is_superuser
3837
and config.SUPERUSER_AUTH_ENFORCEMENT
3938
and obj.has_usable_password()
40-
and not MfaMethod.objects.filter(user=obj, is_active=True).exists()
39+
and not MfaMethodsWrapper.objects.filter(user=obj, is_active=True).exists()
4140
):
4241
return False
4342
return True

hub/tests/test_admin_validators.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
from django.test import TestCase
33

44
from hub.admin.extend_user import validate_superuser_auth
5-
from kobo.apps.accounts.mfa.models import MfaMethod
5+
from kobo.apps.accounts.mfa.models import MfaMethodsWrapper
66
from kobo.apps.kobo_auth.shortcuts import User
77

88

@@ -22,5 +22,5 @@ def test_superuser_with_unusable_password(self):
2222
self.assertTrue(validate_superuser_auth(self.superuser))
2323

2424
def test_superuser_with_mfa_enabled(self):
25-
MfaMethod.objects.create(user=self.superuser, is_active=True)
25+
MfaMethodsWrapper.objects.create(user=self.superuser, is_active=True)
2626
self.assertTrue(validate_superuser_auth(self.superuser))

kobo/apps/__init__.py

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,13 @@
1-
import trench
21
from django.apps import AppConfig
32
from django.core.checks import Tags, register
43

5-
import kpi.utils.monkey_patching # noqa
64
from kpi.utils.two_database_configuration_checker import TwoDatabaseConfigurationChecker
75

86

97
class KpiConfig(AppConfig):
108
name = 'kpi'
119

1210
def ready(self, *args, **kwargs):
13-
# These imports cannot be at the top until the app is loaded.
14-
from kobo.apps.accounts.mfa.command import (
15-
create_mfa_method_command,
16-
deactivate_mfa_method_command,
17-
)
18-
19-
# Monkey-patch `django-trench` to avoid duplicating lots of code in views,
20-
# and serializers just for few line changes.
21-
# Changed behaviours:
22-
# 1. Stop blocking deactivation of primary method
23-
trench.command.deactivate_mfa_method.deactivate_mfa_method_command = (
24-
deactivate_mfa_method_command
25-
)
26-
# 2. Resetting secret on reactivation
27-
trench.command.create_mfa_method.create_mfa_method_command = (
28-
create_mfa_method_command
29-
)
30-
3111
# Load all schema extension modules to register them
3212
import kpi.schema_extensions.imports # noqa F401
3313

kobo/apps/accounts/adapter.py

Lines changed: 0 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -2,22 +2,11 @@
22
from allauth.account.forms import SignupForm
33
from constance import config
44
from django.conf import settings
5-
from django.contrib.auth import REDIRECT_FIELD_NAME, login
65
from django.db import transaction
7-
from django.shortcuts import resolve_url
8-
from django.template.response import TemplateResponse
96
from django.utils import timezone
10-
from trench.utils import get_mfa_model, user_token_generator
11-
12-
from .mfa.forms import MfaTokenForm
13-
from .mfa.models import MfaAvailableToUser
14-
from .mfa.permissions import mfa_allowed_for_user
15-
from .mfa.views import MfaTokenView
16-
from .utils import user_has_inactive_paid_subscription
177

188

199
class AccountAdapter(DefaultAccountAdapter):
20-
2110
def is_open_for_signup(self, request):
2211
return config.REGISTRATION_OPEN
2312

@@ -26,44 +15,6 @@ def login(self, request, user):
2615
user.backend = settings.AUTHENTICATION_BACKENDS[0]
2716
super().login(request, user)
2817

29-
def pre_login(self, request, user, **kwargs):
30-
31-
if parent_response := super().pre_login(request, user, **kwargs):
32-
# A response from the parent means the login process must be
33-
# interrupted, e.g. due to the user being inactive or not having
34-
# validated their email address
35-
return parent_response
36-
37-
# If MFA is activated and allowed for the user, display the token form before letting them in
38-
mfa_active = (
39-
get_mfa_model().objects.filter(is_active=True, user=user).exists()
40-
)
41-
mfa_allowed = mfa_allowed_for_user(user)
42-
inactive_subscription = user_has_inactive_paid_subscription(
43-
user.username
44-
)
45-
if mfa_active and (mfa_allowed or inactive_subscription):
46-
ephemeral_token_cache = user_token_generator.make_token(user)
47-
mfa_token_form = MfaTokenForm(
48-
initial={'ephemeral_token': ephemeral_token_cache}
49-
)
50-
51-
next_url = kwargs.get('redirect_url') or resolve_url(
52-
settings.LOGIN_REDIRECT_URL
53-
)
54-
55-
context = {
56-
REDIRECT_FIELD_NAME: next_url,
57-
'view': MfaTokenView,
58-
'form': mfa_token_form,
59-
}
60-
61-
return TemplateResponse(
62-
request=request,
63-
template='mfa_token.html',
64-
context=context,
65-
)
66-
6718
def save_user(self, request, user, form, commit=True):
6819
# Compare allauth SignupForm with our custom field
6920
standard_fields = set(SignupForm().fields.keys())

kobo/apps/accounts/mfa/adapter.py

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
from typing import Optional
2+
3+
from allauth.mfa.adapter import DefaultMFAAdapter
4+
from allauth.mfa.models import Authenticator
5+
from constance import config
6+
from django.conf import settings
7+
8+
from ..utils import user_has_inactive_paid_subscription
9+
from .models import MfaMethod, MfaMethodsWrapper
10+
from .permissions import mfa_allowed_for_user
11+
12+
13+
class MfaAdapter(DefaultMFAAdapter):
14+
15+
def is_mfa_enabled(self, user, types=None) -> bool:
16+
# NOTE: This is a temporary thing. We are migrating users to the allauth tables
17+
# When the migration is done it won't be necessary.
18+
self.migrate_user(user)
19+
mfa_active_super = super().is_mfa_enabled(user, types)
20+
mfa_active = (
21+
mfa_active_super
22+
and MfaMethodsWrapper.objects.filter(user=user, is_active=True).first()
23+
is not None
24+
)
25+
mfa_allowed = mfa_allowed_for_user(user)
26+
inactive_subscription = user_has_inactive_paid_subscription(user.username)
27+
return mfa_active and (mfa_allowed or inactive_subscription)
28+
29+
def get_totp_label(self, user) -> str:
30+
"""Returns the label used for representing the given user in a TOTP QR
31+
code.
32+
"""
33+
return f'{config.MFA_ISSUER_NAME}-{user.username}'
34+
35+
def get_totp_issuer(self) -> str:
36+
"""Returns the TOTP issuer name that will be contained in the TOTP QR
37+
code.
38+
"""
39+
return config.MFA_ISSUER_NAME
40+
41+
def migrate_user(
42+
self, user: settings.AUTH_USER_MODEL, mfa_method: MfaMethod = None
43+
) -> Optional[MfaMethodsWrapper]:
44+
"""Migrate user MFA data from trench tables to allauth tables"""
45+
if not mfa_method:
46+
mfa_method = (
47+
MfaMethod.objects.filter(name='app', user=user, is_active=True)
48+
.order_by('is_primary')
49+
.first()
50+
)
51+
if not mfa_method:
52+
return
53+
authenticators = Authenticator.objects.filter(user_id=user.id)
54+
types_ok = {a.type for a in authenticators} == {
55+
Authenticator.Type.TOTP,
56+
Authenticator.Type.RECOVERY_CODES,
57+
}
58+
# If allauth MFA Authenticators already exist, exit
59+
if types_ok:
60+
return
61+
for authenticator in authenticators:
62+
authenticator.delete()
63+
64+
encrypted_secret = self.encrypt(mfa_method.secret)
65+
totp_authenticator = Authenticator.objects.create(
66+
user_id=mfa_method.user_id,
67+
type=Authenticator.Type.TOTP,
68+
data={'secret': encrypted_secret},
69+
)
70+
recovery_codes = Authenticator.objects.create(
71+
user_id=mfa_method.user_id,
72+
type=Authenticator.Type.RECOVERY_CODES,
73+
data={
74+
'migrated_codes': [self.encrypt(c) for c in mfa_method.backup_codes],
75+
'used_mask': 0,
76+
},
77+
)
78+
mfa_method_wrapper = MfaMethodsWrapper.objects.create(
79+
name='app',
80+
user=user,
81+
is_active=True,
82+
totp=totp_authenticator,
83+
recovery_codes=recovery_codes,
84+
secret=encrypted_secret,
85+
)
86+
return mfa_method_wrapper

kobo/apps/accounts/mfa/admin.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@
22
from django.contrib import admin
33

44
from .models import (
5-
TrenchMFAMethod,
5+
ExtendedTrenchMfaMethodAdmin,
66
MfaAvailableToUser,
77
MfaAvailableToUserAdmin,
88
MfaMethod,
9-
MfaMethodAdmin,
9+
TrenchMFAMethod,
1010
)
1111

1212
admin.site.unregister(TrenchMFAMethod)
13-
admin.site.register(MfaMethod, MfaMethodAdmin)
13+
admin.site.register(MfaMethod, ExtendedTrenchMfaMethodAdmin)
1414
admin.site.register(MfaAvailableToUser, MfaAvailableToUserAdmin)

kobo/apps/accounts/mfa/apps.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
class MfaAppConfig(AppConfig):
55
name = 'kobo.apps.accounts.mfa'
66
verbose_name = 'Multi-factor authentication'
7+
label = 'accounts_mfa'
78

89
def ready(self):
910
from . import signals # noqa F401

0 commit comments

Comments
 (0)