Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Django All Auth Force MFA Feature #3649

Closed
fish-not-phish opened this issue Feb 19, 2024 · 11 comments
Closed

Django All Auth Force MFA Feature #3649

fish-not-phish opened this issue Feb 19, 2024 · 11 comments

Comments

@fish-not-phish
Copy link

fish-not-phish commented Feb 19, 2024

Hi, I was going through the documentation and I did not see a clear-cut way to force/require users to use MFA when they log in. If there is a way to achieve this that is currently already in place please let me know. I had followed all instructions and configuration recommendations to install/use allauth mfa (pip install, mfa adapater).

However, users have the ability to log in and not even use MFA. This could be a feature some people may need for security reasons. I offer the below decorator that I used to force MFA, but have no impact on any unauthenticated users.

def mfa_required(view_func):
    @wraps(view_func)
    def _wrapped_view(request, *args, **kwargs):
        if request.user.is_authenticated:
            mfa_completed = any(method.get('type') == 'totp' for method in request.session.get('account_authentication_methods', []))
            if not mfa_completed:
                return HttpResponseRedirect('/accounts/2fa/')
        return view_func(request, *args, **kwargs)
    return _wrapped_view

Hopefully this helps some people and I hope to see this potentially added to the allauth package as a future feature :)

@inc-ali
Copy link

inc-ali commented Feb 27, 2024

django-allauth-2fa package has a BaseRequire2FAMiddleware class. I'd love to see something similar for django-allauth

@muazshahid
Copy link

muazshahid commented Mar 13, 2024

@fish-not-phish Could you please provide more details about how you forced MFA on login, I am trying to do the same with an existing app which has social logins setup as well but I want to be able to use MFA for regular email/password login.

@apagano-vue
Copy link

apagano-vue commented Mar 27, 2024

This is what we came up with (adapted from django-allauth-2fa's BaseRequire2FAMiddleware):

from allauth.mfa.utils import is_mfa_enabled
from django.conf import settings
from django.contrib import messages
from django.http import HttpRequest, HttpResponse
from django.shortcuts import redirect
from django.utils.deprecation import MiddlewareMixin

class AllUserRequire2FAMiddleware(MiddlewareMixin):
    """
    Ensure that all users have two-factor authentication enabled before
    they have access to the rest of the app.

    If they don't have 2FA enabled, they will be redirected to the 2FA
    enrollment page and not be allowed to access other pages.
    """

    # List of URL names that the user should still be allowed to access.
    allowed_pages = [
        # They should still be able to log out or change password.
        "account_login",
        "account_logout",
        "account_reauthenticate",
        "account_reset_password_done",
        "account_reset_password_from_key",
        "account_reset_password_from_key_done",
        "account_reset_password",
        "account_email",
        "account_email_verification_sent",
        "account_confirm_email",
        "mfa_activate_totp",
    ]
    # The message to the user if they don't have 2FA enabled and must enable it.
    require_2fa_message = (
        "You must enable two-factor authentication before doing anything else."
    )

    def on_require_2fa(self, request: HttpRequest) -> HttpResponse:
        """
        If the current request requires 2FA and the user does not have it
        enabled, this is executed. The result of this is returned from the
        middleware.
        """
        # See allauth.account.adapter.DefaultAccountAdapter.add_message.
        if "django.contrib.messages" in settings.INSTALLED_APPS:
            # If there is already a pending message related to two-factor (likely
            # created by a redirect view), simply update the message text.
            storage = messages.get_messages(request)
            tag = "2fa_required"
            for m in storage:
                if m.extra_tags == tag:
                    m.message = self.require_2fa_message
                    break
            # Otherwise, create a new message.
            else:
                messages.error(request, self.require_2fa_message, extra_tags=tag)
            # Mark the storage as not processed so they'll be shown to the user.
            storage.used = False

        # Redirect user to two-factor setup page.
        return redirect("mfa_activate_totp")

    def is_allowed_page(self, request: HttpRequest) -> bool:
        return request.resolver_match.url_name in self.allowed_pages

    def process_view(
        self,
        request: HttpRequest,
        view_func,
        view_args,
        view_kwargs,
    ) -> HttpResponse | None:
        # The user is not logged in, do nothing.
        if request.user.is_anonymous:
            return None

        # If the user is on one of the allowed pages, do nothing.
        if self.is_allowed_page(request):
            return None

        # User already has two-factor configured, do nothing.
        if is_mfa_enabled(request.user):
            return None

        # The request required 2FA but it isn't configured!
        return self.on_require_2fa(request)

@pkcakeout
Copy link

Given the fact that #3710 was declared out of scope for the project, should this issue be closed as well? @pennersr

@pennersr
Copy link
Owner

@pkcakeout Correct.

@pennersr
Copy link
Owner

For a bit more context, see: #3710 (comment)

Would having a @mfa_required decorator serve (some) people's needs?

@fish-not-phish
Copy link
Author

@pennersr I think it would. Depending on use case, you can check if the user is authenticated and then apply conditions to check for MFA requirements. I have it built out in my app, I can share it with you if it would help you understand the route I took. It's a bit different than what I have originally posted here.

Essentially, a non-authenticated user can visit specific views, but if you authenticate logon, you will be forced to use MFA. If you try and get around it, you will be redirected to the view to enter the MFA token.

@fish-not-phish
Copy link
Author

fish-not-phish commented Apr 26, 2024

def mfa_required(view_func):
    @wraps(view_func)
    def _wrapped_view(request, *args, **kwargs):
        if request.user.is_authenticated:
            if not Authenticator.objects.filter(user=request.user).exists():
                mfa_completed = any(method.get('type') == 'totp' for method in request.session.get('account_authentication_methods', []))
                if not mfa_completed:
                    return HttpResponseRedirect('/mfa/')
            else: 
                mfa_completed = any(method.get('type') == 'totp' for method in request.session.get('account_authentication_methods', []))
                if not mfa_completed:
                    logout(request)
                    return HttpResponseRedirect(reverse('account_login'))
        return view_func(request, *args, **kwargs)
    return _wrapped_view

Hopefully this is able to give you some context. :)

@jmsmkn
Copy link
Contributor

jmsmkn commented Apr 28, 2024

Would having a @mfa_required decorator serve (some) people's needs?

Our use case is to ensure that anyone who accesses/can access the Django admin application has MFA enabled. Whether it is enforced on user login or on view access does not matter to us.

@eyalch
Copy link
Sponsor Contributor

eyalch commented Jun 22, 2024

In case it's interesting, here's how I override the default admin site to enforce staff users to enable MFA in order to access the admin:

from functools import update_wrapper

from allauth.account.decorators import secure_admin_login
from allauth.mfa.utils import is_mfa_enabled
from django.contrib import messages
from django.contrib.admin import AdminSite
from django.http import HttpResponseRedirect
from django.urls import reverse
from django.utils.decorators import method_decorator
from django.utils.translation import gettext_lazy as _
from django.views.decorators.cache import never_cache


class MyAdminSite(AdminSite):
    def admin_view(self, view, cacheable=False):
        def wrapped(request, *args, **kwargs):
            if not is_mfa_enabled(request.user):
                messages.info(
                    request,
                    _("Please enable two-factor authentication to access the admin."),
                )
                return HttpResponseRedirect(reverse("mfa_index"))
            return view(request, *args, **kwargs)

        wrapped = super().admin_view(wrapped, cacheable=cacheable)
        return update_wrapper(wrapped, view)

    @method_decorator(secure_admin_login)
    @method_decorator(never_cache)
    def login(self, request, extra_context=None):
        return super().login(request, extra_context)

This approach has the advantage of running the check on every admin view, as opposed to just on login. This means that staff users won't be able to disable MFA after logging in and keep using the admin.

@pennersr Should we consider providing a mixin with this functionality? Alternatively, we could just mention it in the docs (probably in the admin page).

@pennersr
Copy link
Owner

@eyalch That would indeed make for a nice improvement. Though, I think whether or not MFA is required should be configurable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants