diff --git a/frontend/__snapshots__/scenes-other-onboarding--onboarding-billing--light.png b/frontend/__snapshots__/scenes-other-onboarding--onboarding-billing--light.png index 93338096ad6c9..95511e628778d 100644 Binary files a/frontend/__snapshots__/scenes-other-onboarding--onboarding-billing--light.png and b/frontend/__snapshots__/scenes-other-onboarding--onboarding-billing--light.png differ diff --git a/frontend/src/scenes/authentication/signup/verify-email/verifyEmailLogic.ts b/frontend/src/scenes/authentication/signup/verify-email/verifyEmailLogic.ts index 55ef8dc85375d..b1b15274f4a44 100644 --- a/frontend/src/scenes/authentication/signup/verify-email/verifyEmailLogic.ts +++ b/frontend/src/scenes/authentication/signup/verify-email/verifyEmailLogic.ts @@ -31,7 +31,7 @@ export const verifyEmailLogic = kea([ { 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([ { 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.' ) diff --git a/posthog/api/email_verification.py b/posthog/api/email_verification.py index 83c12d1dfe1e9..796e4c616d97f 100644 --- a/posthog/api/email_verification.py +++ b/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() diff --git a/posthog/api/test/test_user.py b/posthog/api/test/test_user.py index 4e441679cc709..0d60d7c9c4992 100644 --- a/posthog/api/test/test_user.py +++ b/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,7 +1054,13 @@ 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("otheruser@posthog.com", password="12345678") + assert not self.other_user.is_email_verified + assert not self.other_user.is_email_verified # Email verification request @@ -1062,7 +1068,7 @@ def setUp(self): 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 = "new@posthog.com" + 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 != "new@posthog.com" + assert self.user.pending_email == "new@posthog.com" + + 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 == "new@posthog.com" + assert self.user.pending_email is None + class TestUserTwoFactor(APIBaseTest): def setUp(self): diff --git a/posthog/api/user.py b/posthog/api/user.py index 7fc843b4a5be8..0610abf047df7 100644 --- a/posthog/api/user.py +++ b/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], )