fix: Email validation tokens (#26893)

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

frontend/src/scenes/authentication/signup/verify-email/verifyEmailLogic.ts

@@ -31,7 +31,7 @@ export const verifyEmailLogic = kea<verifyEmailLogicType>([
             {
                 validateEmailToken: async ({ uuid, token }: { uuid: string; token: string }, breakpoint) => {
                     try {
-                        await api.create(`api/users/${uuid}/verify_email/`, { token, uuid })
+                        await api.create(`api/users/verify_email/`, { token, uuid })
                         actions.setView('success')
                         await breakpoint(2000)
                         window.location.href = '/'
@@ -48,7 +48,7 @@ export const verifyEmailLogic = kea<verifyEmailLogicType>([
             {
                 requestVerificationLink: async ({ uuid }: { uuid: string }) => {
                     try {
-                        await api.create(`api/users/${uuid}/request_email_verification/`, { uuid })
+                        await api.create(`api/users/request_email_verification/`, { uuid })
                         lemonToast.success(
                             'A new verification link has been sent to the associated email address. Please check your inbox.'
                         )

posthog/api/email_verification.py

@@ -25,7 +25,9 @@ def _make_hash_value(self, user: AbstractBaseUser, timestamp):
         # Due to type differences between the user model and the token generator, we need to
         # re-fetch the user from the database to get the correct type.
         usable_user: User = User.objects.get(pk=user.pk)
-        return f"{usable_user.pk}{usable_user.email}{usable_user.pending_email}{timestamp}"
+        login_timestamp = "" if user.last_login is None else user.last_login.replace(microsecond=0, tzinfo=None)
+
+        return f"{usable_user.pk}{usable_user.email}{usable_user.pending_email}{login_timestamp}{timestamp}"
 
 
 email_verification_token_generator = EmailVerificationTokenGenerator()

posthog/api/test/test_user.py

@@ -419,7 +419,7 @@ def test_notifications_sent_when_user_email_is_changed_and_email_available(
             token = email_verification_token_generator.make_token(self.user)
             with freeze_time("2020-01-01T21:37:00+00:00"):
                 response = self.client.post(
-                    f"/api/users/@me/verify_email/",
+                    f"/api/users/verify_email/",
                     {"uuid": self.user.uuid, "token": token},
                 )
             self.assertEqual(response.status_code, status.HTTP_200_OK)
@@ -1054,15 +1054,21 @@ def setUp(self):
         # prevent throttling of user requests to pass on from one test
         # to the next
         cache.clear()
-        return super().setUp()
+        super().setUp()
+
+        set_instance_setting("EMAIL_HOST", "localhost")
+
+        self.other_user = self._create_user("[email protected]", password="12345678")
+        assert not self.other_user.is_email_verified
+        assert not self.other_user.is_email_verified
 
     # Email verification request
 
     @patch("posthoganalytics.capture")
     def test_user_can_request_verification_email(self, mock_capture):
         set_instance_setting("EMAIL_HOST", "localhost")
         with self.settings(CELERY_TASK_ALWAYS_EAGER=True, SITE_URL="https://my.posthog.net"):
-            response = self.client.post(f"/api/users/@me/request_email_verification/", {"uuid": self.user.uuid})
+            response = self.client.post(f"/api/users/request_email_verification/", {"uuid": self.user.uuid})
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         self.assertEqual(response.content.decode(), '{"success":true}')
         self.assertSetEqual({",".join(outmail.to) for outmail in mail.outbox}, {self.CONFIG_EMAIL})
@@ -1080,7 +1086,7 @@ def test_user_can_request_verification_email(self, mock_capture):
         reset_link = html_message[link_index : html_message.find('"', link_index)]
         token = reset_link.replace("https://my.posthog.net/verify_email/", "").replace(f"{self.user.uuid}/", "")
 
-        response = self.client.post(f"/api/users/@me/verify_email/", {"uuid": self.user.uuid, "token": token})
+        response = self.client.post(f"/api/users/verify_email/", {"uuid": self.user.uuid, "token": token})
         self.assertEqual(response.status_code, status.HTTP_200_OK)
 
         # check is_email_verified is changed to True
@@ -1114,8 +1120,9 @@ def test_user_can_request_verification_email(self, mock_capture):
         self.assertEqual(mock_capture.call_count, 3)
 
     def test_cant_verify_if_email_is_not_configured(self):
+        set_instance_setting("EMAIL_HOST", "")
         with self.settings(CELERY_TASK_ALWAYS_EAGER=True):
-            response = self.client.post(f"/api/users/@me/request_email_verification/", {"uuid": self.user.uuid})
+            response = self.client.post(f"/api/users/request_email_verification/", {"uuid": self.user.uuid})
         self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
         self.assertEqual(
             response.json(),
@@ -1133,7 +1140,7 @@ def test_cant_verify_more_than_six_times(self):
         for i in range(7):
             with self.settings(CELERY_TASK_ALWAYS_EAGER=True, SITE_URL="https://my.posthog.net"):
                 response = self.client.post(
-                    f"/api/users/@me/request_email_verification/",
+                    f"/api/users/request_email_verification/",
                     {"uuid": self.user.uuid},
                 )
             if i < 6:
@@ -1153,11 +1160,11 @@ def test_cant_verify_more_than_six_times(self):
 
     def test_can_validate_email_verification_token(self):
         token = email_verification_token_generator.make_token(self.user)
-        response = self.client.post(f"/api/users/@me/verify_email/", {"uuid": self.user.uuid, "token": token})
+        response = self.client.post(f"/api/users/verify_email/", {"uuid": self.user.uuid, "token": token})
         self.assertEqual(response.status_code, status.HTTP_200_OK)
 
     def test_cant_validate_email_verification_token_without_a_token(self):
-        response = self.client.post(f"/api/users/@me/verify_email/", {"uuid": self.user.uuid})
+        response = self.client.post(f"/api/users/verify_email/", {"uuid": self.user.uuid})
         self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
         self.assertEqual(
             response.json(),
@@ -1183,7 +1190,7 @@ def test_invalid_verification_token_returns_error(self):
             expired_token,
         ]:
             response = self.client.post(
-                f"/api/users/@me/verify_email/",
+                f"/api/users/verify_email/",
                 {"uuid": self.user.uuid, "token": token},
             )
             self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
@@ -1197,6 +1204,92 @@ def test_invalid_verification_token_returns_error(self):
                 },
             )
 
+    def test_can_only_validate_email_token_one_time(self):
+        token = email_verification_token_generator.make_token(self.user)
+        response = self.client.post(f"/api/users/verify_email/", {"uuid": self.user.uuid, "token": token})
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+
+        response = self.client.post(f"/api/users/verify_email/", {"uuid": self.user.uuid, "token": token})
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+        self.assertEqual(
+            response.json(),
+            {
+                "type": "validation_error",
+                "code": "invalid_token",
+                "detail": "This verification token is invalid or has expired.",
+                "attr": "token",
+            },
+        )
+
+    def test_email_verification_logs_in_user(self):
+        token = email_verification_token_generator.make_token(self.user)
+
+        self.client.logout()
+        assert self.client.get("/api/users/@me/").status_code == 401
+        session_user_id = self.client.session.get("_auth_user_id")
+        assert session_user_id is None
+
+        # NOTE: Posting sets the session user id but doesn't log in the test client hence we just check the session id
+        self.client.post(f"/api/users/verify_email/", {"uuid": self.user.uuid, "token": token})
+        session_user_id = self.client.session.get("_auth_user_id")
+        assert session_user_id == str(self.user.id)
+
+    def test_email_verification_logs_in_correctuser(self):
+        other_token = email_verification_token_generator.make_token(self.other_user)
+        self.client.logout()
+        assert self.client.session.get("_auth_user_id") is None
+
+        # NOTE: The user id in path should basically be ignored
+        self.client.post(f"/api/users/verify_email/", {"uuid": self.other_user.uuid, "token": other_token})
+        session_user_id = self.client.session.get("_auth_user_id")
+        assert session_user_id == str(self.other_user.id)
+
+    def test_email_verification_does_not_apply_to_current_logged_in_user(self):
+        other_token = email_verification_token_generator.make_token(self.other_user)
+
+        res = self.client.post(f"/api/users/verify_email/", {"uuid": self.other_user.uuid, "token": other_token})
+        assert res.status_code == status.HTTP_200_OK
+        self.user.refresh_from_db()
+        self.other_user.refresh_from_db()
+        # Should now be logged in as other user
+        assert self.client.session.get("_auth_user_id") == str(self.other_user.id)
+        assert not self.user.is_email_verified
+        assert self.other_user.is_email_verified
+
+    def test_email_verification_fails_if_using_other_accounts_token(self):
+        token = email_verification_token_generator.make_token(self.user)
+        other_token = email_verification_token_generator.make_token(self.other_user)
+        self.client.logout()
+
+        assert (
+            self.client.post(f"/api/users/verify_email/", {"uuid": self.other_user.uuid, "token": token}).status_code
+            == status.HTTP_400_BAD_REQUEST
+        )
+
+        assert (
+            self.client.post(f"/api/users/verify_email/", {"uuid": self.user.uuid, "token": other_token}).status_code
+            == status.HTTP_400_BAD_REQUEST
+        )
+
+    def test_does_not_apply_pending_email_for_old_tokens(self):
+        self.client.logout()
+
+        token = email_verification_token_generator.make_token(self.user)
+        self.user.pending_email = "[email protected]"
+        self.user.save()
+
+        response = self.client.post(f"/api/users/verify_email/", {"uuid": self.user.uuid, "token": token})
+        assert response.status_code == status.HTTP_400_BAD_REQUEST
+        assert self.user.email != "[email protected]"
+        assert self.user.pending_email == "[email protected]"
+
+        token = email_verification_token_generator.make_token(self.user)
+        response = self.client.post(f"/api/users/verify_email/", {"uuid": self.user.uuid, "token": token})
+        assert response.status_code == status.HTTP_200_OK
+        self.user.refresh_from_db()
+        assert self.user.email == "[email protected]"
+        assert self.user.pending_email is None
+
 
 class TestUserTwoFactor(APIBaseTest):
     def setUp(self):

posthog/api/user.py

@@ -382,10 +382,11 @@ def get_serializer_context(self):
             "user_permissions": UserPermissions(cast(User, self.request.user)),
         }
 
-    @action(methods=["POST"], detail=True, permission_classes=[AllowAny])
+    @action(methods=["POST"], detail=False, permission_classes=[AllowAny])
     def verify_email(self, request, **kwargs):
         token = request.data["token"] if "token" in request.data else None
         user_uuid = request.data["uuid"]
+
         if not token:
             raise serializers.ValidationError({"token": ["This field is required."]}, code="required")
 
@@ -421,7 +422,7 @@ def verify_email(self, request, **kwargs):
 
     @action(
         methods=["POST"],
-        detail=True,
+        detail=False,
         permission_classes=[AllowAny],
         throttle_classes=[UserEmailVerificationThrottle],
     )