Skip to content

Commit

Permalink
feat(account): Allow removal of email depending on auth method
Browse files Browse the repository at this point in the history
  • Loading branch information
pennersr committed Oct 20, 2023
1 parent 0e29d46 commit a0ff1aa
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 32 deletions.
5 changes: 5 additions & 0 deletions ChangeLog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ Note worthy changes

- Officially support Django 5.0.

- In previous versions, users could never remove their primary email address.
This is constraint is now relaxed. In case the email address is not required,
for example, because the user logs in by username, removal of the email
address is allowed.


Backwards incompatible changes
------------------------------
Expand Down
34 changes: 33 additions & 1 deletion allauth/account/adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@

from allauth import app_settings as allauth_app_settings
from allauth.account import signals
from allauth.account.app_settings import EmailVerificationMethod
from allauth.account.app_settings import (
AuthenticationMethod,
EmailVerificationMethod,
)
from allauth.core import context, ratelimit
from allauth.utils import (
build_absolute_uri,
Expand Down Expand Up @@ -88,6 +91,35 @@ def is_email_verified(self, request, email):
ret = verified_email.lower() == email.lower()
return ret

def can_delete_email(self, email_address):
from allauth.account.models import EmailAddress

has_other = (
EmailAddress.objects.filter(user_id=email_address.user_id)
.exclude(pk=email_address.pk)
.exists()
)
login_by_email = (
app_settings.AUTHENTICATION_METHOD == AuthenticationMethod.EMAIL
)
if email_address.primary:
if has_other:
# Don't allow, let the user mark one of the others as primary
# first.
return False
elif login_by_email:
# Last email & login is by email, prevent dangling account.
return False
return True
elif has_other:
# Account won't be dangling.
return True
elif login_by_email:
# This is the last email.
return False
else:
return True

def format_email_subject(self, subject):
prefix = app_settings.EMAIL_SUBJECT_PREFIX
if prefix is None:
Expand Down
85 changes: 57 additions & 28 deletions allauth/account/tests/test_change_email.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,28 +9,11 @@
import pytest
from pytest_django.asserts import assertTemplateNotUsed, assertTemplateUsed

from allauth.account.app_settings import AuthenticationMethod
from allauth.account.models import EmailAddress, EmailConfirmationHMAC
from allauth.account.utils import user_email


# class ChangeEmailTests(TestCase):
# def setUp(self):
# User = get_user_model()
# self.user = User.objects.create(username="john", email="[email protected]")
# self.user.set_password("doe")
# self.user.save()
# self.email_address = EmailAddress.objects.create(
# user=self.user, email=self.user.email, verified=True, primary=True
# )
# self.email_address2 = EmailAddress.objects.create(
# user=self.user,
# email="[email protected]",
# verified=False,
# primary=False,
# )
# self.client.login(username="john", password="doe")


def test_ajax_get(auth_client, user):
primary = EmailAddress.objects.filter(user=user).first()
secondary = EmailAddress.objects.create(
Expand Down Expand Up @@ -76,16 +59,8 @@ def test_ajax_add_invalid(auth_client):
assert "valid" in data["form"]["fields"]["email"]["errors"][0]


def test_remove_primary(auth_client, user):
resp = auth_client.post(
reverse("account_email"),
{"action_remove": "", "email": user.email},
)
assert EmailAddress.objects.filter(email=user.email).exists()
assertTemplateUsed(resp, "account/messages/cannot_delete_primary_email.txt")


def test_ajax_remove_primary(auth_client, user):
def test_ajax_remove_primary(auth_client, user, settings):
settings.ACCOUNT_AUTHENTICATION_METHOD = "email"
resp = auth_client.post(
reverse("account_email"),
{"action_remove": "", "email": user.email},
Expand Down Expand Up @@ -320,3 +295,57 @@ def test_add_not_allowed(
assert resp.context["form"].errors == {
"email": ["A user is already registered with this email address."]
}


@pytest.mark.parametrize(
"authentication_method,primary_email,secondary_emails,delete_email,success",
[
(AuthenticationMethod.EMAIL, "pri@mail", ["sec@mail"], "pri@mail", False),
(AuthenticationMethod.EMAIL, "pri@mail", ["sec@mail"], "sec@mail", True),
(AuthenticationMethod.EMAIL, "pri@mail", [], "pri@mail", False),
(AuthenticationMethod.USERNAME, "pri@mail", ["sec@mail"], "pri@mail", False),
(AuthenticationMethod.USERNAME, "pri@mail", ["sec@mail"], "sec@mail", True),
(AuthenticationMethod.USERNAME, "pri@mail", [], "pri@mail", True),
(
AuthenticationMethod.USERNAME_EMAIL,
"pri@mail",
["sec@mail"],
"pri@mail",
False,
),
(
AuthenticationMethod.USERNAME_EMAIL,
"pri@mail",
["sec@mail"],
"sec@mail",
True,
),
(AuthenticationMethod.USERNAME_EMAIL, "pri@mail", [], "pri@mail", True),
],
)
def test_remove_email(
client,
settings,
user_factory,
primary_email,
secondary_emails,
delete_email,
authentication_method,
success,
):
settings.ACCOUNT_AUTHENTICATION_METHOD = authentication_method
user = user_factory(email=primary_email)
EmailAddress.objects.bulk_create(
[
EmailAddress(user=user, email=email, primary=False, verified=False)
for email in secondary_emails
]
)
client.force_login(user)
resp = client.post(
reverse("account_email"),
{"action_remove": "", "email": delete_email},
)
assert EmailAddress.objects.filter(email=delete_email).exists() == (not success)
if not success:
assertTemplateUsed(resp, "account/messages/cannot_delete_primary_email.txt")
7 changes: 4 additions & 3 deletions allauth/account/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -523,8 +523,9 @@ def _action_send(self, request, *args, **kwargs):
def _action_remove(self, request, *args, **kwargs):
email_address = self._get_email_address(request)
if email_address:
if email_address.primary:
get_adapter().add_message(
adapter = get_adapter()
if not adapter.can_delete_email(email_address):
adapter.add_message(
request,
messages.ERROR,
"account/messages/cannot_delete_primary_email.txt",
Expand All @@ -538,7 +539,7 @@ def _action_remove(self, request, *args, **kwargs):
user=request.user,
email_address=email_address,
)
get_adapter().add_message(
adapter.add_message(
request,
messages.SUCCESS,
"account/messages/email_deleted.txt",
Expand Down

0 comments on commit a0ff1aa

Please sign in to comment.