iMerica / dj-rest-auth

Authentication for Django Rest Framework
https://dj-rest-auth.readthedocs.io/en/latest/index.html
MIT License
1.67k stars 310 forks source link

Error with ACCOUNT_EMAIL_VERIFICATION = 'mandatory' and ACCOUNT_CONFIRM_EMAIL_ON_GET = True #138

Open brunomichetti opened 4 years ago

brunomichetti commented 4 years ago

Hello I'm using dj-rest-auth in my app with mandatory email account confirmation, and account confirmation when click on the link received in the email.

Doing the GET to the generated link this error raises:

TemplateResponseMixin requires either a definition of 'template_name' or an implementation of 'get_template_names()'

Here I show you a screenshot: Screen Shot 2020-09-02 at 15 49 14

I've found a solution, that is adding this in my urls:

path(
        'rest-auth/registration/account-confirm-email/<str:key>/',
        ConfirmEmailView.as_view(),
    )

It works if I add that path BEFORE the path('rest-auth/registration/', include('dj_rest_auth.registration.urls')),. Otherwise it raises the mentioned error.

Is there a better solution? Do you know how can be this issue solved?

jerinpetergeorge commented 3 years ago

Could you add the ConfirmEmailView class? It seems like the issue belongs to your view class itself. @brunomichetti

brunomichetti commented 3 years ago

Could you add the ConfirmEmailView class? It seems like the issue belongs to your view class itself. @brunomichetti

Thanks @jerinpetergeorge for the response. ConfirmEmailView is the one from dj_rest_auth. I import the view from dj_rest_auth.registration.views. I think the problem is in that view class too

jerinpetergeorge commented 3 years ago

from the codebase

    # account_confirm_email - You should override this view to handle it in
    # your API client somehow and then, send post to /verify-email/ endpoint
    # with proper key.
    # If you don't want to use API on that step, then just use ConfirmEmailView
    # view from:
    # django-allauth https://github.com/pennersr/django-allauth/blob/master/allauth/account/views.py
    re_path(r'^account-confirm-email/(?P<key>[-:\w]+)/$', TemplateView.as_view(),
        name='account_confirm_email'),

So, when you are using path('rest-auth/registration/', include('dj_rest_auth.registration.urls')) in your code, the dj-rest-auth will use the Django's TemplateView class for the account confirmation. But, in your case, you are using ConfirmEmailView which is originally defined in the all-auth package. @brunomichetti

brunomichetti commented 3 years ago

from the codebase

    # account_confirm_email - You should override this view to handle it in
    # your API client somehow and then, send post to /verify-email/ endpoint
    # with proper key.
    # If you don't want to use API on that step, then just use ConfirmEmailView
    # view from:
    # django-allauth https://github.com/pennersr/django-allauth/blob/master/allauth/account/views.py
    re_path(r'^account-confirm-email/(?P<key>[-:\w]+)/$', TemplateView.as_view(),
        name='account_confirm_email'),

So, when you are using _path('rest-auth/registration/', include('dj_rest_auth.registration.urls'))_ in your code, the dj-rest-auth will use the Django's TemplateView class for the account confirmation. But, in your case, you are using ConfirmEmailView which is originally defined in the all-auth package. @brunomichetti

Yes, but the registration should work without the need of adding that path, shouldn't? I added that path because the normal configuration isn't working

jerinpetergeorge commented 3 years ago

As the comment indicates, if you are using the dj_rest_auth.registration.urls for the "account confirmation", the process will not get completed as the URL pointing to a dummy view, which does nothing.

Update

@brunomichetti I have managed to recreate the issue.

If you are getting an exception as

TemplateResponseMixin requires either a definition of 'template_name' or an implementation of 'get_template_names()'

which means you are still requesting to the rest auth's account-confirm-email view, not to the ConfirmEmailView.

brunomichetti commented 3 years ago

As the comment indicates, if you are using the dj_rest_auth.registration.urls for the "account confirmation", the process will not get completed as the URL pointing to a dummy view, which does nothing.

Update

@brunomichetti I have managed to recreate the issue.

If you are getting an exception as

TemplateResponseMixin requires either a definition of 'template_name' or an implementation of 'get_template_names()'

which means you are still requesting to the rest auth's account-confirm-email view, not to the ConfirmEmailView.

Thank you for the help. Don't you think that if you set ACCOUNT_EMAIL_VERIFICATION = 'mandatory' and ACCOUNT_CONFIRM_EMAIL_ON_GET = True the get to the generated url should validate and return a valid answer without the need of adding that path?

jerinpetergeorge commented 3 years ago

Those settings are irrelevant here.

You can try any arbitrary key (ex: /rest-auth/registration/account-confirm-email/foobar/) to access the URL and it will give you some other response as below,

image

brunomichetti commented 3 years ago

Those settings are irrelevant here.

You can try any arbitrary key (ex: /rest-auth/registration/account-confirm-email/foobar/) to access the URL and it will give you some other response as below,

image

Sorry, but I don't understand what you mean. I want to:

  1. set email confirmation as mandatory
  2. if I do a GET to the url received on the email then receive a valid response, not the error.

Is this right now possible with dj-rest-auth without the need of adding the path?

jerinpetergeorge commented 3 years ago

No, that is not possible without adding the path

brunomichetti commented 3 years ago

No, that is not possible without adding the path

Do you agree with me that a fix should be done to avoid the adding of the path?

jerinpetergeorge commented 3 years ago

That's a nice suggestion!!!

jerinpetergeorge commented 3 years ago

I have created a separate issue here. Let's hope we will be able to see it on the master branch soon.

brunomichetti commented 3 years ago

I have created a separate issue here. Let's hope we will be able to see it on the master branch soon.

Thanks a lot!!!! Hope this can be addressed soon