Skip to content

Commit

Permalink
fix(socialaccount): Prevent enumeration at social signup form
Browse files Browse the repository at this point in the history
  • Loading branch information
pennersr committed Jul 8, 2023
1 parent 9a390e7 commit f58889c
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 17 deletions.
4 changes: 4 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,7 @@ isort:
.PHONY: black
black:
black allauth/ setup.py

.PHONY: test
test:
pytest allauth/
27 changes: 19 additions & 8 deletions allauth/account/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,24 @@ def custom_signup(self, request, user):
# in case of ModelForm
custom_form.save(user)

def try_save(self, request):
"""Try and save te user. This can fail in case of a conflict on the
email address, in that case we will send an "account already exists"
email and return a standard "email verification sent" response.
"""
if self.account_already_exists:
# Don't create a new acount, only send an email informing the user
# that (s)he already has one...
email = self.cleaned_data["email"]
adapter = get_adapter(request)
adapter.send_account_already_exists_mail(email)
user = None
resp = adapter.respond_email_verification_sent(request, None)
else:
user = self.save(request)
resp = None
return user, resp


class SignupForm(BaseSignupForm):
def __init__(self, *args, **kwargs):
Expand Down Expand Up @@ -435,10 +453,7 @@ def clean(self):

def save(self, request):
if self.account_already_exists:
# Don't create a new acount, only send an email informing the user
# that (s)he already has one...
self._send_account_already_exists_mail(request)
return
raise ValueError(self.cleaned_data.get("email"))
adapter = get_adapter(request)
user = adapter.new_user(request)
adapter.save_user(request, user, self)
Expand All @@ -447,10 +462,6 @@ def save(self, request):
setup_user_email(request, user, [])
return user

def _send_account_already_exists_mail(self, request):
email = self.cleaned_data["email"]
get_adapter(request).send_account_already_exists_mail(email)


class UserForm(forms.Form):
def __init__(self, user=None, *args, **kwargs):
Expand Down
8 changes: 3 additions & 5 deletions allauth/account/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -245,11 +245,9 @@ def get_success_url(self):
return ret

def form_valid(self, form):
self.user = form.save(self.request)
if not self.user:
return get_adapter(self.request).respond_email_verification_sent(
self.request, None
)
self.user, resp = form.try_save(self.request)
if resp:
return resp
try:
return complete_signup(
self.request,
Expand Down
4 changes: 2 additions & 2 deletions allauth/socialaccount/tests/test_signup.py
Original file line number Diff line number Diff line change
Expand Up @@ -456,8 +456,8 @@ def test_email_address_conflict_at_social_signup_form(
session.save()
# Here, we input the already existing email.
resp = client.post(reverse("socialaccount_signup"), {"email": user.email})
# TODO: This is wrong -- prevent enumeration should kick in.
assert mailoutbox[0].subject == "[example.com] Please Confirm Your E-mail Address"
assert mailoutbox[0].subject == "[example.com] Account Already Exists"
assert resp["location"] == reverse("account_email_verification_sent")


def test_email_address_conflict_during_auto_signup(
Expand Down
6 changes: 4 additions & 2 deletions allauth/socialaccount/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,10 @@ def get_form_kwargs(self):

def form_valid(self, form):
self.request.session.pop("socialaccount_sociallogin", None)
form.save(self.request)
return helpers.complete_social_signup(self.request, self.sociallogin)
user, resp = form.try_save(self.request)
if not resp:
resp = helpers.complete_social_signup(self.request, self.sociallogin)
return resp

def get_context_data(self, **kwargs):
ret = super(SignupView, self).get_context_data(**kwargs)
Expand Down

0 comments on commit f58889c

Please sign in to comment.