jazzband / django-invitations

Generic invitations app for Django
GNU General Public License v3.0
562 stars 166 forks source link

django-allauth-2fa and django-invitations #157

Closed asucrews closed 2 months ago

asucrews commented 3 years ago

Hello, I was wondering if anyone got django-allauth-2fa to work with django-invitations?

I am trying myself, but I can't figure out what changes need to be made to the account adapter. Below is my custom account adapter. With this, I don't get any error messages, but it doesn't seem to load settings correctly. mainly the invitation_only option does not seem to work correctly.

Oh, I have also tested allauth-2fa and it works without invitations, and invitations works without allauth-2fa.

Thanks!

-- INVITATIONS_INVITATION_EXPIRY = 7 INVITATIONS_INVITATION_ONLY = True INVITATIONS_ACCEPT_INVITE_AFTER_SIGNUP = True INVITATIONS_ADAPTER = 'core.adapter.BaseInvitationsAdapter'

from django.conf import settings from django.contrib import messages from django.contrib.sites.models import Site from django.core.mail import EmailMessage, EmailMultiAlternatives from django.template.exceptions import TemplateDoesNotExist from django.utils.encoding import force_text from django.template.loader import render_to_string from allauth.account.signals import user_signed_up from invitations.app_settings import app_settings from allauth_2fa.adapter import DefaultAccountAdapter

class OTPAdapter(DefaultAccountAdapter):

def has_2fa_enabled(self, user):
    """Returns True if the user has 2FA configured."""
    return user_has_valid_totp_device(user)

def login(self, request, user):
    # Require two-factor authentication if it has been configured.
    if self.has_2fa_enabled(user):
        # Cast to string for the case when this is not a JSON serializable
        # object, e.g. a UUID.
        request.session['allauth_2fa_user_id'] = str(user.id)

        redirect_url = reverse('two-factor-authenticate')
        # Add "next" parameter to the URL.
        view = request.resolver_match.func.view_class()
        view.request = request
        success_url = view.get_success_url()
        query_params = request.GET.copy()
        if success_url:
            query_params[view.redirect_field_name] = success_url
        if query_params:
            redirect_url += '?' + urlencode(query_params)

        raise ImmediateHttpResponse(
            response=HttpResponseRedirect(redirect_url)
        )

    # Otherwise defer to the original allauth adapter.
    return super(OTPAdapter, self).login(request, user)

def is_open_for_signup(self, request):
    if hasattr(request, 'session') and request.session.get(
            'account_verified_email'):
        return True
    elif app_settings.INVITATION_ONLY is True:
        # Site is ONLY open for invites
        return False
    else:
        # Site is open to signup
        return True

def get_user_signed_up_signal(self):
    return user_signed_up

class BaseInvitationsAdapter(object): def stash_verified_email(self, request, email): request.session['account_verified_email'] = email

def unstash_verified_email(self, request):
    ret = request.session.get('account_verified_email')
    request.session['account_verified_email'] = None
    return ret

def format_email_subject(self, subject):
    prefix = app_settings.EMAIL_SUBJECT_PREFIX
    if prefix is None:
        site = Site.objects.get_current()
        prefix = "[{name}] ".format(name=site.name)
    return prefix + force_text(subject)

def render_mail(self, template_prefix, email, context):
    """
    Renders an e-mail to `email`.  `template_prefix` identifies the
    e-mail that is to be sent, e.g. "account/email/email_confirmation"
    """
    subject = render_to_string('{0}_subject.txt'.format(template_prefix),
                               context)
    # remove superfluous line breaks
    subject = " ".join(subject.splitlines()).strip()
    subject = self.format_email_subject(subject)

    bodies = {}
    for ext in ['html', 'txt']:
        try:
            template_name = '{0}_message.{1}'.format(template_prefix, ext)
            bodies[ext] = render_to_string(template_name,
                                           context).strip()
        except TemplateDoesNotExist:
            if ext == 'txt' and not bodies:
                # We need at least one body
                raise
    if 'txt' in bodies:
        msg = EmailMultiAlternatives(subject,
                                     bodies['txt'],
                                     settings.DEFAULT_FROM_EMAIL,
                                     [email])
        if 'html' in bodies:
            msg.attach_alternative(bodies['html'], 'text/html')
    else:
        msg = EmailMessage(subject,
                           bodies['html'],
                           settings.DEFAULT_FROM_EMAIL,
                           [email])
        msg.content_subtype = 'html'  # Main content is now text/html
    return msg

def send_mail(self, template_prefix, email, context):
    msg = self.render_mail(template_prefix, email, context)
    msg.send()

def is_open_for_signup(self, request):
    if hasattr(request, 'session') and request.session.get(
            'account_verified_email'):
        return True
    elif app_settings.INVITATION_ONLY is True:
        # Site is ONLY open for invites
        return False
    else:
        # Site is open to signup
        return True

def clean_email(self, email):
    """
    Validates an email value. You can hook into this if you want to
    (dynamically) restrict what email addresses can be chosen.
    """
    return email

def add_message(self, request, level, message_template,
                message_context=None, extra_tags=''):
    """
    Wrapper of `django.contrib.messages.add_message`, that reads
    the message text from a template.
    """
    if 'django.contrib.messages' in settings.INSTALLED_APPS:
        try:
            if message_context is None:
                message_context = {}
            message = render_to_string(message_template,
                                       message_context).strip()
            if message:
                messages.add_message(request, level, message,
                                     extra_tags=extra_tags)
        except TemplateDoesNotExist:
            pass

def get_user_signed_up_signal(self):
    return user_signed_up
pbadeer commented 3 years ago

I have got this working using purely other people's code, I just combined the account adapter's from both of these packages.

settings.py ACCOUNT_ADAPTER = 'myapp.adapter.ComboAdapter'

repo/myapp/adapter.py

from allauth_2fa.adapter import OTPAdapter
from allauth.account.signals import user_signed_up
from django.conf import settings

# Subclassing OTPAdapter gives us allauth_2fa functions
class ComboAdapter(OTPAdapter):

    # Could not attach these functions directly due to the logic here
    # https://github.com/bee-keeper/django-invitations/blob/master/invitations/models.py#L76
    # These are copied from InvitationsAdapter from django-invitations
    def is_open_for_signup(self, request):
        if hasattr(request, 'session') and request.session.get(
                'account_verified_email'):
            return True
        elif settings.INVITATION_ONLY is True:
            # Site is ONLY open for invites
            return False
        else:
            # Site is open to signup
            return True

    def get_user_signed_up_signal(self):
        return user_signed_up

Since I'm hard-copying functions from InvitationsAdapter into ComboAdapter, please make sure you do a fresh copy yourself if you're viewing this in the future. Go to https://github.com/bee-keeper/django-invitations/blob/master/invitations/models.py and copy anything inside of InvitationsAdapter into ComboAdapter.

In the future, if the logic I mentioned in my code comment above is removed and InvitationsAdapter is available as a class at all times, then it would be much cleaner (and future-proof) to do this:

# THIS SNIPPET IS A SUGGESTION FOR THE FUTURE, use the code above for now
from allauth_2fa.adapter import OTPAdapter
from invitations.models import InvitationsAdapter

class ComboAdapter(OTPAdapter, InvitationsAdapter):
    pass
clokep commented 3 years ago

I think we had this working at some point at percipient, I don't have the code handy though. I believe #30 actually was used to fix this case.

I think your suggested code should pretty much work, you can then set ACCOUNT_ADAPTER = "path.to.ComboAdapter".

pbadeer commented 3 years ago

Yup that's what this was doing:

ACCOUNT_ADAPTER = 'myapp.adapter.ComboAdapter'

30 ensures that you can retrieve the active adapter (rather than None) but does not fix the issue of it failing to detect if the active adapter is a subclass of InvitationsAdapter. It cannot detect this (nor can it allow you to subclass) because InvitationsAdapter only exists if your ACCOUNT_ADAPTER is set specifically to invitations.models.InvitationsAdapter.

It would be great if this class https://github.com/bee-keeper/django-invitations/blob/master/invitations/models.py#L81 was always instantiated even when not in use. 🙏🏻

clokep commented 3 years ago

It cannot detect this (nor can it allow you to subclass) because InvitationsAdapter only exists if your ACCOUNT_ADAPTER is set specifically to invitations.models.InvitationsAdapter.

Ah...yeah that's familiar now. I believe we ended up on the same solution as you -- essentially vendoring that code into our application. I think your proposed solution of always having that code exist makes sense. 👍

asucrews commented 3 years ago

just woundering if i model was made always available?

pbadeer commented 2 years ago

@asucrews No, the latest version of django-invitations still has not changed this approach.

pbadeer commented 1 year ago

Note if you're getting the error: AttributeError: 'function' object has no attribute 'view_class'

This was an issue in django-allauth-2fa and was recently fixed on the main branch but has not been released yet. https://github.com/valohai/django-allauth-2fa/issues/143

asucrews commented 1 year ago

Just checking in to see if this was made possible?

Flimm commented 3 months ago

allauth supports two factor authentication out of the box now. Let's close this issue, unless there are people still interested in integrating django-allauth-2fa.

pbadeer commented 2 months ago

An organization I work with helped fund the development of the allauth native 2fa feature, in hopes that it would be better maintained than the allauth-2fa library. We're migrating and recommend others do too.

asucrews commented 2 months ago

Closing, native is better in this case.