Skip to content

Commit

Permalink
fix(headless/account): LOGIN_ON_EMAIL_CONFIRMATION not respected
Browse files Browse the repository at this point in the history
  • Loading branch information
pennersr committed Dec 12, 2024
1 parent 842b0d9 commit 3a7bba4
Show file tree
Hide file tree
Showing 10 changed files with 122 additions and 18 deletions.
3 changes: 3 additions & 0 deletions ChangeLog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ Fixes
- Headless: In contrast to the headed version, it was possible to remove the
last 3rd party account from a user that has no usable password. Fixed.

- Headless: The setting ``ACCOUNT_LOGIN_ON_EMAIL_CONFIRMATION`` was not respected,
and always assumed to be ``True``.


65.3.0 (2024-11-30)
*******************
Expand Down
8 changes: 6 additions & 2 deletions allauth/account/internal/flows/email_verification_by_code.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,16 @@ def key_expired(self):
def confirm(self, request) -> Optional[EmailAddress]:
ret = super().confirm(request)
if ret:
clear_state(request)
clear_pending_verification(request)
return ret


def clear_state(request):
def clear_pending_verification(request):
request.session.pop(EMAIL_VERIFICATION_CODE_SESSION_KEY, None)


def clear_state(request):
clear_pending_verification(request)
clear_login(request)


Expand Down
12 changes: 9 additions & 3 deletions allauth/account/tests/test_email_verification_by_code.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ def test_signup(
get_last_email_verification_code,
query,
expected_url,
mailoutbox,
):
password = password_factory()
resp = client.post(
Expand All @@ -45,7 +46,7 @@ def test_signup(
},
)
assert get_user_model().objects.filter(username="johndoe").count() == 1
code = get_last_email_verification_code()
code = get_last_email_verification_code(client, mailoutbox)
assert resp.status_code == 302
assert resp["location"] == reverse("account_email_verification_sent")
resp = client.get(reverse("account_email_verification_sent"))
Expand Down Expand Up @@ -88,7 +89,12 @@ def test_signup_prevent_enumeration(

@pytest.mark.parametrize("change_email", (False, True))
def test_add_or_change_email(
auth_client, user, get_last_email_verification_code, change_email, settings
auth_client,
user,
get_last_email_verification_code,
change_email,
settings,
mailoutbox,
):
settings.ACCOUNT_CHANGE_EMAIL = change_email
email = "[email protected]"
Expand All @@ -102,7 +108,7 @@ def test_add_or_change_email(
assert not email_added_signal.send.called
assert not email_changed_signal.send.called
assert EmailAddress.objects.filter(email=email).count() == 0
code = get_last_email_verification_code()
code = get_last_email_verification_code(auth_client, mailoutbox)
resp = auth_client.get(reverse("account_email_verification_sent"))
assert resp.status_code == 200
with patch("allauth.account.signals.email_added") as email_added_signal:
Expand Down
14 changes: 9 additions & 5 deletions allauth/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -328,17 +328,21 @@ def get(self, path):


@pytest.fixture
def get_last_email_verification_code(client, mailoutbox):
def get_last_email_verification_code():
from allauth.account.internal.flows import email_verification_by_code

def f():
def f(client, mailoutbox):
code = re.search(
"\n[0-9a-z]{6}\n", mailoutbox[0].body, re.I | re.DOTALL | re.MULTILINE
)[0].strip()
if hasattr(client, "headless_session"):
session = client.headless_session()
else:
session = client.session
assert (
client.session[
email_verification_by_code.EMAIL_VERIFICATION_CODE_SESSION_KEY
]["code"]
session[email_verification_by_code.EMAIL_VERIFICATION_CODE_SESSION_KEY][
"code"
]
== code
)
return code
Expand Down
16 changes: 14 additions & 2 deletions allauth/headless/account/tests/test_email_verification.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import pytest

from allauth.account.models import (
EmailAddress,
EmailConfirmationHMAC,
Expand All @@ -23,9 +25,19 @@ def test_verify_email_other_user(auth_client, user, user_factory, headless_rever
assert data["data"]["user"]["id"] == user.pk


@pytest.mark.parametrize(
"login_on_email_verification,status_code", [(False, 401), (True, 200)]
)
def test_auth_unverified_email(
client, user_factory, password_factory, settings, headless_reverse
client,
user_factory,
password_factory,
settings,
headless_reverse,
login_on_email_verification,
status_code,
):
settings.ACCOUNT_LOGIN_ON_EMAIL_CONFIRMATION = login_on_email_verification
settings.ACCOUNT_AUTHENTICATION_METHOD = "email"
settings.ACCOUNT_EMAIL_VERIFICATION = "mandatory"
password = password_factory()
Expand All @@ -49,7 +61,7 @@ def test_auth_unverified_email(
data={"key": key},
content_type="application/json",
)
assert resp.status_code == 200
assert resp.status_code == status_code


def test_verify_email_bad_key(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
from unittest.mock import ANY

from django.contrib.auth.models import User

import pytest

from allauth.account import app_settings
Expand Down Expand Up @@ -116,6 +120,7 @@ def test_add_email(
headless_reverse,
settings,
get_last_email_verification_code,
mailoutbox,
):
settings.ACCOUNT_AUTHENTICATION_METHOD = "email"
settings.ACCOUNT_EMAIL_VERIFICATION_BY_CODE_ENABLED = True
Expand Down Expand Up @@ -150,7 +155,7 @@ def test_add_email(
}

# And with the valid code...
code = get_last_email_verification_code()
code = get_last_email_verification_code(auth_client, mailoutbox)
resp = auth_client.post(
headless_reverse("headless:account:verify_email"),
data={"key": code},
Expand All @@ -169,3 +174,65 @@ def test_add_email(
content_type="application/json",
)
assert resp.status_code == 400


@pytest.mark.parametrize(
"login_on_email_verification,status_code", [(False, 401), (True, 200)]
)
def test_signup_with_email_verification(
db,
client,
email_factory,
password_factory,
settings,
headless_reverse,
headless_client,
get_last_email_verification_code,
login_on_email_verification,
status_code,
mailoutbox,
):
settings.ACCOUNT_EMAIL_VERIFICATION = "mandatory"
settings.ACCOUNT_USERNAME_REQUIRED = False
settings.ACCOUNT_LOGIN_ON_EMAIL_CONFIRMATION = login_on_email_verification
settings.ACCOUNT_EMAIL_VERIFICATION_BY_CODE_ENABLED = True
email = email_factory()
resp = client.post(
headless_reverse("headless:account:signup"),
data={
"email": email,
"password": password_factory(),
},
content_type="application/json",
)
assert resp.status_code == 401
assert User.objects.filter(email=email).exists()
data = resp.json()
flow = next((f for f in data["data"]["flows"] if f.get("is_pending")))
assert flow["id"] == "verify_email"

code = get_last_email_verification_code(client, mailoutbox)
resp = client.get(
headless_reverse("headless:account:verify_email"),
HTTP_X_EMAIL_VERIFICATION_KEY=code,
)
assert resp.status_code == 200
assert resp.json() == {
"data": {
"email": email,
"user": ANY,
},
"meta": {"is_authenticating": True},
"status": 200,
}
resp = client.post(
headless_reverse("headless:account:verify_email"),
data={"key": code},
content_type="application/json",
)
addr = EmailAddress.objects.get(email=email)
assert addr.verified

assert resp.status_code == status_code
data = resp.json()
assert data["meta"]["is_authenticated"] is login_on_email_verification
1 change: 1 addition & 0 deletions allauth/headless/account/tests/test_signup.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ def test_signup_with_email_verification(
):
settings.ACCOUNT_EMAIL_VERIFICATION = "mandatory"
settings.ACCOUNT_USERNAME_REQUIRED = False
settings.ACCOUNT_LOGIN_ON_EMAIL_CONFIRMATION = True
email = email_factory()
resp = client.post(
headless_reverse("headless:account:signup"),
Expand Down
11 changes: 6 additions & 5 deletions allauth/headless/account/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from allauth.account.adapter import get_adapter as get_account_adapter
from allauth.account.internal import flows
from allauth.account.internal.flows import (
email_verification,
manage_email,
password_change,
password_reset,
Expand Down Expand Up @@ -164,16 +165,16 @@ def get(self, request, *args, **kwargs):
return response.VerifyEmailResponse(request, verification, stage=self.stage)

def post(self, request, *args, **kwargs):
confirmation = self.input.cleaned_data["key"]
email_address = confirmation.confirm(request)
verification = self.input.cleaned_data["key"]
email_address = verification.confirm(request)
if not email_address:
# Should not happen, VerifyInputInput should have verified all
# preconditions.
return APIResponse(request, status=500)
if self.stage:
# Verifying email as part of login/signup flow, so emit a
# authentication status response.
self.stage.exit()
# Verifying email as part of login/signup flow may imply the user is
# to be logged in...
email_verification.login_on_verification(request, verification)
return AuthenticationResponse(request)


Expand Down
1 change: 1 addition & 0 deletions allauth/headless/mfa/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ def test_auth_unverified_email_and_mfa(
):
settings.ACCOUNT_AUTHENTICATION_METHOD = "email"
settings.ACCOUNT_EMAIL_VERIFICATION = "mandatory"
settings.ACCOUNT_LOGIN_ON_EMAIL_CONFIRMATION = True
password = password_factory()
user = user_factory(email_verified=False, password=password, with_totp=True)
resp = client.post(
Expand Down
5 changes: 5 additions & 0 deletions docs/headless/openapi-specification/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,11 @@ paths:
their email address, or, by inputting a code that is sent. On the API,
both cases are handled identically. Meaning, the required key is either
the one from the link, or, the code itself.
Note that a status code of 401 does not imply failure. It indicates that
the email verification was successful, yet, the user is still not signed
in. For example, in case `ACCOUNT_LOGIN_ON_EMAIL_CONFIRMATION` is set to
`False`, a 401 is returned when verifying as part of login/signup.
parameters:
- $ref: "#/components/parameters/Client"
- $ref: "#/components/parameters/SessionToken"
Expand Down

0 comments on commit 3a7bba4

Please sign in to comment.