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

feat(account): Move RequestPasswordReset.save impl into account adapter #4034

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion allauth/account/adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -563,9 +563,19 @@ def is_safe_url(self, url):

return url_has_allowed_host_and_scheme(url, allowed_hosts=allowed_hosts)

def send_password_reset_mail(self, user, email, context):
"""
Method intended to be overridden in case you need to customize the logic
used to determine whether a user is permitted to request a password reset.
For example, if you are enforcing something like "social only" authentication
in your app, you may want to intervene here by checking `user.has_usable_password`

"""
return self.send_mail("account/email/password_reset_key", email, context)

def get_reset_password_from_key_url(self, key):
"""
Method intented to be overriden in case the password reset email
Method intended to be overridden in case the password reset email
needs to be adjusted.
"""
from allauth.account.internal import flows
Expand Down
18 changes: 8 additions & 10 deletions allauth/account/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from allauth.utils import get_username_max_length, set_form_field_order

from . import app_settings
from .adapter import get_adapter
from .adapter import DefaultAccountAdapter, get_adapter
from .app_settings import AuthenticationMethod
from .models import EmailAddress, Login
from .utils import (
Expand Down Expand Up @@ -598,18 +598,15 @@ def clean_email(self):
raise get_adapter().validation_error("unknown_email")
return self.cleaned_data["email"]

def save(self, request, **kwargs):
def save(self, request, **kwargs) -> str:
email = self.cleaned_data["email"]
if not self.users:
flows.signup.send_unknown_account_mail(request, email)
else:
self._send_password_reset_mail(request, email, self.users, **kwargs)
return email
return email

def _send_password_reset_mail(self, request, email, users, **kwargs):
adapter: DefaultAccountAdapter = get_adapter()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be good to either abort the method inside the if above by returning return email, or, to reintroduce the else. As is now, both cases are executed (the if and the else part), and while that works because users is empty it did trip me over while reviewing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, good catch though!

token_generator = kwargs.get("token_generator", default_token_generator)

for user in users:
for user in self.users:
temp_key = token_generator.make_token(user)

# send the password reset email
Expand All @@ -618,7 +615,7 @@ def _send_password_reset_mail(self, request, email, users, **kwargs):
# not implementation details such as a separate `uidb36` and
# `key. Ideally, this should have done on `urls` level as well.
key = f"{uid}-{temp_key}"
url = get_adapter().get_reset_password_from_key_url(key)
url = adapter.get_reset_password_from_key_url(key)
context = {
"user": user,
"password_reset_url": url,
Expand All @@ -629,7 +626,8 @@ def _send_password_reset_mail(self, request, email, users, **kwargs):

if app_settings.AUTHENTICATION_METHOD != AuthenticationMethod.EMAIL:
context["username"] = user_username(user)
get_adapter().send_mail("account/email/password_reset_key", email, context)
adapter.send_password_reset_mail(user, email, context)
pennersr marked this conversation as resolved.
Show resolved Hide resolved
return email


class ResetPasswordKeyForm(PasswordVerificationMixin, forms.Form):
Expand Down