Skip to content

Commit

Permalink
Merge branch 'main' into portal-frontend-70
Browse files Browse the repository at this point in the history
  • Loading branch information
faucomte97 authored Jan 30, 2025
2 parents 34b64ce + d51394c commit ca7f6b0
Show file tree
Hide file tree
Showing 12 changed files with 156 additions and 82 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
## [1.0.14](https://github.com/ocadotechnology/codeforlife-portal-backend/compare/v1.0.13...v1.0.14) (2025-01-28)


### Bug Fixes

* Add user to AuthFactor serializer, fix email address change flow ([#375](https://github.com/ocadotechnology/codeforlife-portal-backend/issues/375)) ([c56e3d9](https://github.com/ocadotechnology/codeforlife-portal-backend/commit/c56e3d9a1288e9e4b563fa120942f8206170897c))

## [1.0.13](https://github.com/ocadotechnology/codeforlife-portal-backend/compare/v1.0.12...v1.0.13) (2025-01-20)


Expand Down
3 changes: 2 additions & 1 deletion settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@
"Invite teacher - account exists": 1569599,
"User already registered": 1569539,
"Teacher released from school": 1569537,
"Email change notification": 1551600,
"Email will change": 1551600,
"Email has changed": 1695716,
"Verify changed user email": 1551594,
"Account deletion": 1567477,
"Inactive users on website - first reminder": 1604381,
Expand Down
10 changes: 6 additions & 4 deletions src/api/auth/token_generators.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,14 @@ def _get_audience(self, user_or_pk: t.Union[User, t.Any]):
pk = user_or_pk.pk if isinstance(user_or_pk, User) else user_or_pk
return f"user:{pk}"

def make_token(self, user_or_pk: t.Union[User, t.Any]):
def make_token(self, user_or_pk: t.Union[User, t.Any], email: str):
"""Generate a token used to verify user's email address.
https://pyjwt.readthedocs.io/en/stable/usage.html
Args:
user: The user to generate a token for.
email: The user's email address to be verified.
Returns:
A token used to verify user's email address.
Expand All @@ -46,6 +47,7 @@ def make_token(self, user_or_pk: t.Union[User, t.Any]):
+ timedelta(seconds=settings.EMAIL_VERIFICATION_TIMEOUT)
),
"aud": [self._get_audience(user_or_pk)],
"email": email,
},
key=settings.SECRET_KEY,
algorithm="HS256",
Expand All @@ -63,7 +65,7 @@ def check_token(self, user_or_pk: t.Union[User, t.Any], token: str):
expired.
"""
try:
jwt.decode(
decoded_jwt = jwt.decode(
jwt=token,
key=settings.SECRET_KEY,
audience=self._get_audience(user_or_pk),
Expand All @@ -74,9 +76,9 @@ def check_token(self, user_or_pk: t.Union[User, t.Any], token: str):
jwt.ExpiredSignatureError,
jwt.InvalidAudienceError,
):
return False
return None

return True
return decoded_jwt


email_verification_token_generator = EmailVerificationTokenGenerator()
2 changes: 2 additions & 0 deletions src/api/serializers/auth_factor.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,11 @@ class Meta:
"id",
"type",
"otp",
"user",
]
extra_kwargs = {
"id": {"read_only": True},
"user": {"read_only": True},
}

# pylint: disable-next=missing-function-docstring
Expand Down
43 changes: 39 additions & 4 deletions src/api/serializers/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ def create(self, validated_data):
kwargs={
"pk": independent_user.pk,
"token": email_verification_token_generator.make_token(
independent_user.pk
independent_user.pk, validated_data["email"]
),
},
)
Expand Down Expand Up @@ -282,6 +282,33 @@ def update(self, instance, validated_data):
# TODO: Send email in signal to teacher of selected class to
# notify them of join request.

email = validated_data.pop("email", None)
if email is not None and email.lower() != instance.email.lower():
send_mail(
settings.DOTDIGITAL_CAMPAIGN_IDS["Email will change"],
to_addresses=[instance.email],
personalization_values={"NEW_EMAIL_ADDRESS": email},
)

# pylint: disable-next=duplicate-code
verify_email_address_link = settings.SERVICE_BASE_URL + reverse(
"user-verify-email-address",
kwargs={
"pk": instance.pk,
"token": email_verification_token_generator.make_token(
instance.pk, email
),
},
)

send_mail(
settings.DOTDIGITAL_CAMPAIGN_IDS["Verify changed user email"],
to_addresses=[email],
personalization_values={
"VERIFICATION_LINK": verify_email_address_link
},
)

return super().update(instance, validated_data)


Expand Down Expand Up @@ -426,20 +453,28 @@ def validate_token(self, value: str):
"Can only verify the email address of an existing user.",
code="user_does_not_exist",
)
if not email_verification_token_generator.check_token(

token = email_verification_token_generator.check_token(
self.instance, value
):
)
if token is None:
raise serializers.ValidationError(
"Does not match the given user.",
code="does_not_match",
)

return value
return token

def update(self, instance, validated_data):
instance.userprofile.is_verified = True
instance.userprofile.save(update_fields=["is_verified"])

email = validated_data["token"]["email"]
if email.lower() != instance.email.lower():
instance.email = email
instance.username = email
instance.save(update_fields=["email", "username"])

return instance


Expand Down
75 changes: 69 additions & 6 deletions src/api/serializers/user_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@
"""

import typing as t
from copy import deepcopy
from datetime import date
from unittest.mock import Mock, patch
from unittest.mock import Mock, call, patch

from codeforlife.tests import ModelSerializerTestCase
from codeforlife.user.models import (
Expand All @@ -16,10 +17,15 @@
StudentUser,
User,
)
from django.conf import settings
from django.contrib.auth.hashers import make_password
from django.db.models import Count
from django.urls import reverse

from ..auth import password_reset_token_generator
from ..auth import (
email_verification_token_generator,
password_reset_token_generator,
)
from .user import (
BaseUserSerializer,
CreateUserSerializer,
Expand Down Expand Up @@ -264,7 +270,7 @@ def test_validate_requesting_to_join_class__no_longer_accepts_requests(
error_code="no_longer_accepts_requests",
)

def test_update(self):
def test_update__requesting_to_join_class(self):
"""Can update the class an independent user is requesting join."""
self.assert_update(
instance=self.indy_user,
Expand All @@ -276,6 +282,56 @@ def test_update(self):
new_data={"new_student": {"pending_class_request": self.class_2}},
)

@patch("src.api.serializers.user.send_mail")
def test_update__email(self, send_mail: Mock):
"""Requesting to update the email field sends a notification email to
the old address and a verification email to the new address."""
instance = self.admin_school_teacher_user
previous_email = instance.email
new_email = "[email protected]"
validated_data = {"email": new_email}

serializer = self._init_model_serializer()

with patch.object(
email_verification_token_generator,
"make_token",
return_value=email_verification_token_generator.make_token(
instance, new_email
),
) as make_token:
serializer.update(instance, deepcopy(validated_data))

make_token.assert_called_once_with(instance.pk, new_email)

send_mail.assert_has_calls(
[
call(
settings.DOTDIGITAL_CAMPAIGN_IDS["Email will change"],
to_addresses=[previous_email],
personalization_values={"NEW_EMAIL_ADDRESS": new_email},
),
call(
settings.DOTDIGITAL_CAMPAIGN_IDS[
"Verify changed user email"
],
to_addresses=[new_email],
personalization_values={
"VERIFICATION_LINK": (
settings.SERVICE_BASE_URL
+ reverse(
"user-verify-email-address",
kwargs={
"pk": instance.pk,
"token": make_token.return_value,
},
)
)
},
),
]
)


class TestHandleIndependentUserJoinClassRequestSerializer(
ModelSerializerTestCase[User, IndependentUser]
Expand Down Expand Up @@ -470,11 +526,18 @@ def test_validate_token__does_not_match(self):
)

def test_update(self):
"""Can successfully reset a user's password."""
"""Can successfully verify a user's email."""
email = "[email protected]"

self.assert_update(
instance=self.user,
validated_data={},
new_data={"userprofile": {"is_verified": True}},
non_model_fields={"token"},
validated_data={"token": {"email": email}},
new_data={
"userprofile": {"is_verified": True},
"email": email,
"username": email,
},
)


Expand Down
2 changes: 1 addition & 1 deletion src/api/signals/student.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def student__post_save(
kwargs={
"pk": instance.new_user.pk,
"token": email_verification_token_generator.make_token(
instance.new_user.pk
instance.new_user.pk, instance.new_user.email
),
},
)
Expand Down
8 changes: 6 additions & 2 deletions src/api/signals/student_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ def test_post_save(self, send_mail: Mock):
student.class_field = None

email_verification_token = (
email_verification_token_generator.make_token(student.new_user.pk)
email_verification_token_generator.make_token(
student.new_user.pk, student.new_user.email
)
)

with patch.object(
Expand All @@ -49,7 +51,9 @@ def test_post_save(self, send_mail: Mock):
) as make_token:
student.save(update_fields=["class_field"])

make_token.assert_called_once_with(student.new_user.pk)
make_token.assert_called_once_with(
student.new_user.pk, student.new_user.email
)

send_mail.assert_called_once_with(
settings.DOTDIGITAL_CAMPAIGN_IDS["Verify released student email"],
Expand Down
21 changes: 2 additions & 19 deletions src/api/signals/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ def user__post_save(
kwargs={
"pk": instance.pk,
"token": email_verification_token_generator.make_token(
instance.pk
instance.pk, instance.email
),
},
)
Expand Down Expand Up @@ -115,28 +115,11 @@ def user__post_save(
)

send_mail(
settings.DOTDIGITAL_CAMPAIGN_IDS["Email change notification"],
settings.DOTDIGITAL_CAMPAIGN_IDS["Email has changed"],
to_addresses=[previous_email],
personalization_values={"NEW_EMAIL_ADDRESS": instance.email},
)

verify_email_address_link = settings.SERVICE_BASE_URL + reverse(
"user-verify-email-address",
kwargs={
"pk": instance.pk,
"token": email_verification_token_generator.make_token(
instance.pk
),
},
)

send_mail(
settings.DOTDIGITAL_CAMPAIGN_IDS["Verify changed user email"],
to_addresses=[instance.email],
personalization_values={
"VERIFICATION_LINK": verify_email_address_link
},
)
# TODO: remove in new schema
elif (
instance.email == ""
Expand Down
Loading

0 comments on commit ca7f6b0

Please sign in to comment.