feedback

src/api/auth/token_generators.py

@@ -28,15 +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], new_email: str = ""):
+    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.
-            new_email: The user's new email address to be verified, if updating
-            it.
+            email: The user's email address to be verified.
 
         Returns:
             A token used to verify user's email address.
@@ -48,7 +47,7 @@ def make_token(self, user_or_pk: t.Union[User, t.Any], new_email: str = ""):
                     + timedelta(seconds=settings.EMAIL_VERIFICATION_TIMEOUT)
                 ),
                 "aud": [self._get_audience(user_or_pk)],
-                "new_email": new_email,
+                "email": email,
             },
             key=settings.SECRET_KEY,
             algorithm="HS256",
@@ -66,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),
@@ -77,28 +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
-
-    def get_new_email(self, user_or_pk: t.Union[User, t.Any], token: str):
-        """Retrieve token's new_email value.
-
-        Args:
-            user: The user to check.
-            token: The token to check.
-
-        Returns:
-            The token's new_email value.
-        """
-        decoded_jwt = jwt.decode(
-            jwt=token,
-            key=settings.SECRET_KEY,
-            audience=self._get_audience(user_or_pk),
-            algorithms=["HS256"],
-        )
-
-        return decoded_jwt["new_email"]
+        return decoded_jwt
 
 
 email_verification_token_generator = EmailVerificationTokenGenerator()

src/api/serializers/user.py

@@ -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"]
                 ),
             },
         )
@@ -282,12 +282,12 @@ def update(self, instance, validated_data):
                 # TODO: Send email in signal to teacher of selected class to
                 #  notify them of join request.
 
-        new_email = validated_data.pop("email", None)
-        if new_email is not None and new_email != instance.email:
+        email = validated_data.pop("email", None)
+        if email is not None and email.lower() != instance.email.lower():
             send_mail(
                 settings.DOTDIGITAL_CAMPAIGN_IDS["Email change notification"],
                 to_addresses=[instance.email],
-                personalization_values={"NEW_EMAIL_ADDRESS": new_email},
+                personalization_values={"NEW_EMAIL_ADDRESS": email},
             )
 
             # pylint: disable-next=duplicate-code
@@ -296,14 +296,14 @@ def update(self, instance, validated_data):
                 kwargs={
                     "pk": instance.pk,
                     "token": email_verification_token_generator.make_token(
-                        instance.pk, new_email=new_email
+                        instance.pk, email
                     ),
                 },
             )
 
             send_mail(
                 settings.DOTDIGITAL_CAMPAIGN_IDS["Verify changed user email"],
-                to_addresses=[new_email],
+                to_addresses=[email],
                 personalization_values={
                     "VERIFICATION_LINK": verify_email_address_link
                 },
@@ -453,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
 
 

src/api/serializers/user_test.py

@@ -297,12 +297,12 @@ def test_update__email(self, send_mail: Mock):
             email_verification_token_generator,
             "make_token",
             return_value=email_verification_token_generator.make_token(
-                instance, new_email=new_email
+                instance, new_email
             ),
         ) as make_token:
             serializer.update(instance, deepcopy(validated_data))
 
-            make_token.assert_called_once_with(instance.pk, new_email=new_email)
+            make_token.assert_called_once_with(instance.pk, new_email)
 
             send_mail.assert_has_calls(
                 [
@@ -528,7 +528,7 @@ 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."""
         self.assert_update(
             instance=self.user,
             validated_data={},

src/api/signals/student.py

@@ -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
                 ),
             },
         )

src/api/signals/student_test.py

@@ -39,7 +39,7 @@ 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(
@@ -49,7 +49,7 @@ 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"],

src/api/signals/user.py

@@ -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
                     ),
                 },
             )

src/api/views/user.py

@@ -184,14 +184,6 @@ def verify_email_address(self, request: Request, **url_params: str):
         serializer.is_valid(raise_exception=True)
         serializer.save()
 
-        new_email = email_verification_token_generator.get_new_email(
-            user, url_params["token"]
-        )
-        if new_email != "":
-            user.email = new_email
-            user.username = new_email
-            user.save()
-
         return Response(
             status=status.HTTP_303_SEE_OTHER,
             headers={
@@ -261,7 +253,7 @@ def _send_verify_email_reminder(self, days: int, campaign_name: str):
                     kwargs={
                         "pk": user_fields["id"],
                         "token": email_verification_token_generator.make_token(
-                            user_fields["id"]
+                            user_fields["id"], user_fields["email"]
                         ),
                     },
                 )

src/api/views/user_test.py

@@ -403,42 +403,21 @@ def test_verify_email_address(self):
         user = User.objects.filter(userprofile__is_verified=False).first()
         assert user
 
-        self.client.get(
-            self.reverse_action(
-                "verify_email_address",
-                model=user,
-                kwargs={
-                    "token": email_verification_token_generator.make_token(user)
-                },
-            ),
-            status_code_assertion=status.HTTP_303_SEE_OTHER,
-        )
-
-        user.refresh_from_db()
-        assert user.userprofile.is_verified
-
-    def test_verify_new_email_address(self):
-        """Can verify a user's new email address following email change
-        request."""
-        user = User.objects.filter(userprofile__is_verified=True).first()
-        assert user
-
         new_email = "[email protected]"
 
         self.client.get(
             self.reverse_action(
                 "verify_email_address",
                 model=user,
                 kwargs={
-                    "token": email_verification_token_generator.make_token(
-                        user, new_email=new_email
-                    )
+                    "token": email_verification_token_generator.make_token(user, new_email)
                 },
             ),
             status_code_assertion=status.HTTP_303_SEE_OTHER,
         )
 
         user.refresh_from_db()
+        assert user.userprofile.is_verified
         assert user.email == new_email
         assert user.username == new_email
 
@@ -480,7 +459,7 @@ def test_create(
 
         add_contact_to_dot_digital.assert_called_once()
 
-        make_token.assert_called_once_with(user_id)
+        make_token.assert_called_once_with(user_id, data["email"])
 
         send_mail_mock.assert_called_once_with(
             campaign_id=settings.DOTDIGITAL_CAMPAIGN_IDS[
@@ -620,7 +599,7 @@ def test_send_verify_email_reminder(
                     if mail_sent:
                         make_token.assert_has_calls(
                             [
-                                call(user.id)
+                                call(user.id, user.email)
                                 for user in teacher_users + indy_users
                             ],
                             any_order=True,