pennersr / django-allauth

Integrated set of Django applications addressing authentication, registration, account management as well as 3rd party (social) account authentication.
https://allauth.org
MIT License
9.55k stars 3.04k forks source link

Recursion Loop on rendering E-Mail #2915

Closed georgkrause closed 2 years ago

georgkrause commented 3 years ago

Hello everyone. At first, thanks for the great, ongoing work!

I have the following problem: When we try to send an Confirmation E-Mail using Django-allauth, this email gets rendered. Since version 0.43 render_to_string gets the request passed. This causes Django to try to render the response for the request, which might run into authentication, which might fail due to an not verified mail address. Now django-allauth tries to resend an confirmation email, and we are in an indefinite recursion loop.

From my point of view the problem is caused by this line: https://github.com/pennersr/django-allauth/blob/master/allauth/account/adapter.py#L119

I am wondering if there is any use in passing the request into the rendering of the E-Mail. Do we need it? If yes, how can we prevent to run into this infinite loop? Or am I doing something totally wrong? I would be happy about any pointers!

Thanks and have a nice time! :)

9mido commented 3 years ago

Can you provide a traceback and the code that you wrote? Can you describe the exact steps you did to re-create this problem in more detail? How are you sending the confirmation email (signups, re-send confirmation button, login with unconfirmed email addresses etc)? What are your allauth settings? Try debugging each line of code you wrote with the print().

georgkrause commented 3 years ago

@9mido thanks for taking care! :)

Its a little bit complicated to actually show the code. Its embedded into a bigger open source project and has a lot of interdependent parts involved. Anyway, I can quite easy share a traceback since its part of our Pipeline:

https://dev.funkwhale.audio/funkwhale/funkwhale/-/pipelines/15388/test_report

It is perfectly working with version 0.42, but when I do the upgrade to 0.43 or 0.44 it fails.

The test case which fails is available here: https://dev.funkwhale.audio/funkwhale/funkwhale/-/blob/develop/api/tests/users/oauth/test_views.py#L410

What we are trying is to authenticate with an unverified mail address while the verification is mandatory. We expect this to return HTTP status code 401. The request gets bootstrapped and fired.

I think the relevant code it self is mostly located here.

We are raising a UnverifiedEmail exception, which gets caught here. Now django-allauth tries to resend the verification E-Mail, but since the request is passed down to the rendering, Django (or Django Rest Framework) handles this as a normal http request, which causes the hole thing go into an infinitive loop.

I am not sure why you added the passing of the request to the email renderer in 0.43, though, but I suppose this is the root cause for our problem.

Thanks for any help and your work!

JuniorJPDJ commented 3 years ago

Bump!

pennersr commented 2 years ago

Closing -- the bug report is unclear. Feel free to reopen, but do provide a small example project demonstrating the issue with plain allauth.

georgkrause commented 11 months ago

@pennersr This is still relevant and I have no option to reopen the issue. Please do it, I'll try to provide more information.

The problem seems to be solved if I don't pass the request down to EmailAddress.send_confirmation(), maybe this provides some pointer?

pennersr commented 11 months ago

@georgkrause I really don't think this is an issue in allauth. This really does not add up:

this email gets rendered. Since version 0.43 render_to_string gets the request passed. This causes Django to try to render the response for the request, which might run into authentication

Django will indeed render a template, but rendering a template and sending the email using the result will (normally) not result in authentication kicking in.

Unless there is something more tangible to go on, we should not reopen this issue. From your stack trace I can tell that there are other parts involved, such as rest framework and funkwhale_api/common/authentication.py -- you will most likely find your answer there.

pennersr commented 11 months ago

One thing that looks suspicious from that stack trace is that the request passed to the render_to_string() seems not to be the regular Django request, but a rest_framework.Request instance which is the one invoking self._authenticate(). I think that request originates from here:

funkwhale_api/common/authentication.py:45: in authenticate
    resend_confirmation_email(request, e.user)
funkwhale_api/common/authentication.py:34: in resend_confirmation_email
    done = send_email_confirmation(request, user)

Be sure to send a regular Django request down that method.

georgkrause commented 11 months ago

@pennersr Thanks for all the information, this helps a lot!