Graphene-Django 3.0 incompatible with Allauth>= 0.48.0 Login forms for testing purposes

[elementace]

Not sure if this is a bug or a feature or even the right place. I have some basic tests set up for testing a SignIn mutation that my Django Backend servers over JWT/DRF and GraphQL that worked fine with 2.15 of this library. As of GD 3.0 and allauth 0.48.0 this seems to be broken.

email = "user@example.org"
password = uuid.uuid4().hex
query_2 = f"""mutation {{
signIn(input: {{
    login: "{email}",
    password: "{password}"
    }}) {{
      errors {{ field messages }}
    }}
}}"""
response_2 = self.query(query=query_2)
content_2 = response_2.json()
expected_2 = {"data": {"signIn": {"errors": []}}}  # type: ignore
self.assertResponseNoErrors(resp=response_2)

This fails with the error: 'NoneType' object has no attribute 'method'

This comes from the new ratelimit functionality introduced in django-allauth 0.48.0. The LoginForm expects a request object when created; the ratelimit consume functionality checks for a request.method, even before cleaning the form data.

https://github.com/pennersr/django-allauth/blob/master/allauth/ratelimit.py

def consume(request, *, action, key=None, amount=None, duration=None, user=None):
    allowed = True
    from allauth.account import app_settings

    rate = app_settings.RATE_LIMITS.get(action)
    if rate:
        rate = parse(rate)
        if not amount:
            amount = rate.amount
        if not duration:
            duration = rate.duration

    if request.method == "GET" or not amount or not duration:
        pass

https://github.com/pennersr/django-allauth/blob/master/allauth/account/forms.py

class LoginForm(forms.Form):

    password = PasswordField(label=_("Password"), autocomplete="current-password")
    remember = forms.BooleanField(label=_("Remember Me"), required=False)

    user = None
    error_messages = {
        "account_inactive": _("This account is currently inactive."),
        "email_password_mismatch": _(
            "The e-mail address and/or password you specified are not correct."
        ),
        "username_password_mismatch": _(
            "The username and/or password you specified are not correct."
        ),
    }

    def __init__(self, *args, **kwargs):
        self.request = kwargs.pop("request", None)

I believe this can be fixed by changing the get_form_kwargs method of the BaseDjangoFormMutation in graphene-django.mutation to add info.context as a 'request' entry to the kwargs dictionary:

@classmethod
  def get_form_kwargs(cls, root, info, **input):
       kwargs = {"data": input}

to

      kwargs = {
          "data": input,
          "request": info.context
      }
      ....
      return kwargs

For now if anyone else comes across this; downgrade your allauth to 0.47.0 and you'll be fine.

My relevant versions: django=3.2.16 django-allauth==0.51.0 django-cors-headers==3.13.0 django-extensions==3.2.1 django-graphql-extensions==0.1.0 django-graphql-jwt==0.3.4 graphene-django==3.0.0 graphql-core==3.2.3

[melvyn-apryl]

Not sure I get it. Who calls consume?

Seems to me, that self.request on the form can be None, so if that is provided to ratelimit.consume() it should grow a guard and turn into a no-op. Did you try adding info.context in get_form_kwargs to see if that fixes things?

[elementace]

Unfortunately I added info.context into get_form_kwargs, but it broke a separate allauth form that wasn't expecting request as an arg :(

I think I agree, this is an allauth bug ultimately; like you said they should form a guard. Is it ok to leave this issue here, closed so I can refer to it externally?

The full stack is: We check if the form is valid in BaseDjangoFormMutation

def mutate_and_get_payload(cls, root, info, **input):
    form = cls.get_form(root, info, **input)

    if form.is_valid():

This triggers BaseForm in django.forms.forms to do a full_clean through the self.errors method

    def is_valid(self):
        """Return True if the form has no errors, or False otherwise."""
        return self.is_bound and not self.errors

    def errors(self):
        """Return an ErrorDict for the data provided for the form."""
        if self._errors is None:
            self.full_clean()
        return self._errors

full_clean runs self._clean_form()

    def full_clean(self):
        """
        Clean all of self.data and populate self._errors and self.cleaned_data.
        """
        self._errors = ErrorDict()
        if not self.is_bound:  # Stop further processing.
            return
        self.cleaned_data = {}
        # If the form is permitted to be empty, and none of the form data has
        # changed from the initial data, short circuit any validation.
        if self.empty_permitted and not self.has_changed():
            return

        self._clean_fields()
        self._clean_form()

    def _clean_form(self):
        try:
            cleaned_data = self.clean()

which gets down to LoginForm for Allauth.account.forms running an authentication

    def clean(self):
        super(LoginForm, self).clean()
        if self._errors:
            return
        credentials = self.user_credentials()
        user = get_adapter(self.request).authenticate(self.request, **credentials)

https://github.com/pennersr/django-allauth/blob/master/allauth/account/adapter.py

def authenticate(self, request, **credentials):
        """Only authenticates, does not actually login. See `login`"""
        from allauth.account.auth_backends import AuthenticationBackend

        self.pre_authenticate(request, **credentials)

that calls pre-authenticate which unfortunately checks the rate limit

def pre_authenticate(self, request, **credentials):
        if app_settings.LOGIN_ATTEMPTS_LIMIT:
            cache_key = self._get_login_attempts_cache_key(request, **credentials)
            if not ratelimit.consume(
                request,

And finally consume fails because we are asking for the '.method' of a Nonetype

def consume(request, *, action, key=None, amount=None, duration=None, user=None):
    allowed = True
    from allauth.account import app_settings

    rate = app_settings.RATE_LIMITS.get(action)
    if rate:
        rate = parse(rate)
        if not amount:
            amount = rate.amount
        if not duration:
            duration = rate.duration

    if request.method == "GET" or not amount or not duration:

Because when we instantiated the form, we didn't pass it a request kwarg

@classmethod
    def get_form(cls, root, info, **input):
        form_kwargs = cls.get_form_kwargs(root, info, **input)
        return cls._meta.form_class(**form_kwargs)

    @classmethod
    def get_form_kwargs(cls, root, info, **input):
        kwargs = {"data": input}

        pk = input.pop("id", None)
        if pk:
            instance = cls._meta.model._default_manager.get(pk=pk)
            kwargs["instance"] = instance

        return kwargs

and it filled it with a NoneType instead (and didn't handle it properly).

    def __init__(self, *args, **kwargs):
        self.request = kwargs.pop("request", None)

[melvyn-apryl]

So allauth handles this in the view. In this case, it makes sense to disable rate limiting and implement this in the mutation. You may be able to call the same consume() method, from the mutation in the same way the adapter does, but using different settings so you short-circuit the call in all-auth's LoginForm.

[elementace]

That makes sense, I'll rig something up to make the tests work. Thanks @melvyn-apryl