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

Add request to email.send_confirmation() in ResendEmailVerificationView #316

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MoBoo
Copy link

@MoBoo MoBoo commented Oct 11, 2021

Why?
Currently the email activation which is send when you request to resend the email verification link, the new link uses the domain specified in Djangos Sites package in the database. This links domain therefore might not be the same as the original email verification link.

For example:
Original email verification link: http://localhost:8000/verify-email/<key>/
Resend email verification link: http://example.com/verify-email/<key>/

This could be easily

Proposed Fix
Add request as parameter to email.send_confirmation() in ResendEmailVerificationView to allow url of the current website to be the target url for the email verification link (see below).

Debugging Trace

  • ResendEmailVerificationView (line 127)
  • EmailAddress.send_confirmation (allauth/account/models.py line 57)
  • EmailConfirmation.send (allauth/account/models.py line 123)
  • DefaultAccountAdapter.send_confirmation_mail (allauth/account/adapter.py line 451)
  • DefaultAccountAdapter.get_email_confirmation_url (allauth/account/adapter.py line 440)
  • build_absolute_uri (allauth/utils.py line 265) (see below)
def build_absolute_uri(request, location, protocol=None):
[...]
    if request is None:
        site = Site.objects.get_current() <-- this is what happens, which fetches the website domain from the database
        [...]
    else:
        uri = request.build_absolute_uri(location) <-- this would be expected, because we could pass the request from the ResendEmailVerificationView and therefore use the domain the user used to request resend

**Why?**
Currently the email activation which is send when you request to resend the email verification link, the new link uses the domain specified in Djangos Sites package in the database. This links domain therefore might not be the same as the original email verification link.

For example:
Original email verification link: `http://localhost:8000/verify-email/<key>/`
Resend email verification link: `http://example.com/verify-email/<key>/`

This could be easily 

**Proposed Fix**
Add `request` as parameter to `email.send_confirmation()` in `ResendEmailVerificationView` to allow url of the current website to be the target url for the email verification link (see below).

**Debugging Trace**
- ResendEmailVerificationView (line 127)
- EmailAddress.send_confirmation (allauth/account/models.py line 57)
- EmailConfirmation.send  (allauth/account/models.py line 123)
- DefaultAccountAdapter.send_confirmation_mail (allauth/account/adapter.py line 451)
- DefaultAccountAdapter.get_email_confirmation_url (allauth/account/adapter.py line 440)
- build_absolute_uri (allauth/utils.py line 265) (see below)
```
def build_absolute_uri(request, location, protocol=None):
[...]
    if request is None:
        site = Site.objects.get_current() <-- this is what happens, which fetches the website domain from the database
        [...]
    else:
        uri = request.build_absolute_uri(location) <-- this would be expected, because we could pass the request from the ResendEmailVerificationView and therefore use the domain the user used to request resend
```
@dannylessio
Copy link

I'm facing the same issue. I'm using the sites framework using SiteManager.get_current() method, that calls SiteManager._get_site_by_request(request) only if request objects is passed. So this endpoint gives me an error, because SiteManager._get_site_by_request(request) cannot be called.

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

Successfully merging this pull request may close these issues.

2 participants