valohai / django-allauth-2fa

Two-factor authentication for Django Allauth
Other
212 stars 51 forks source link

Consider where django-allauth-2fa stands now django-allauth has MFA built-in #189

Open valberg opened 9 months ago

valberg commented 9 months ago

django-allauth has support for MFA since 0.56.0 - maybe this means that django-allauth-2fa has served it's purpose?

See comment by @pennersr here: https://github.com/pennersr/django-allauth/issues/3420#issuecomment-1711553191

There is definitely a timeline where django-allauth-2fa has a justification to be kept alive - but I'm not sure if we are living in it.

What do you think @akx ?

akx commented 9 months ago

Yeah, I guess an integrated solution will be better (and less maintenance for us, although more for @pennersr).

I suppose we'd need to

akx commented 9 months ago

Copying @pennersr's experimental migration management command here – we have his blessing to use this in this project:

import base64

from allauth.mfa.adapter import get_adapter
from allauth.mfa.models import Authenticator
from django.core.management.base import BaseCommand
from django_otp.plugins.otp_static.models import StaticDevice
from django_otp.plugins.otp_totp.models import TOTPDevice

class Command(BaseCommand):
    def handle(self, **options):
        adapter = get_adapter()
        authenticators = []
        for totp in TOTPDevice.objects.filter(confirmed=True).iterator():
            recovery_codes = set()
            for sdevice in StaticDevice.objects.filter(confirmed=True, user_id=totp.user_id).iterator():
                recovery_codes.update(sdevice.token_set.values_list("token", flat=True))
            secret = base64.b32encode(bytes.fromhex(totp.key)).decode("ascii")
            totp_authenticator = Authenticator(
                user_id=totp.user_id,
                type=Authenticator.Type.TOTP,
                data={"secret": adapter.encrypt(secret)},
            )
            authenticators.append(totp_authenticator)
            authenticators.append(
                Authenticator(
                    user_id=totp.user_id,
                    type=Authenticator.Type.RECOVERY_CODES,
                    data={
                        "migrated_codes": [adapter.encrypt(c) for c in recovery_codes],
                    },
                )
            )
        Authenticator.objects.bulk_create(authenticators)
valberg commented 9 months ago

Could we provide the above code as the last migration in django-allauth-2fa? I might be missing something and it won't be a feasible way to go.

jmsmkn commented 2 months ago

We are looking into migrating to the 2FA solution in django-allauth. As far as we can tell the only missing feature for us would be a way of requiring 2FA for certain users, which is implemented in django-allauth-2fa.

We made a PR to add this to django-allauth but despite other users also using this feature it was declared out of scope. It is not clear to us what a "clear de facto way of handling" this feature is, but, I think it would be valuable to many people to maintain this feature somewhere, be it in django-allauth, django-allauth-2fa, or somewhere else.