Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Email validation tokens #26893

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -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 = '/'
Expand All @@ -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 })
MichaelKutsch-ph marked this conversation as resolved.
Show resolved Hide resolved
lemonToast.success(
'A new verification link has been sent to the associated email address. Please check your inbox.'
)
Expand Down
4 changes: 3 additions & 1 deletion posthog/api/email_verification.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
111 changes: 102 additions & 9 deletions posthog/api/test/test_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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})
Expand All @@ -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
Expand Down Expand Up @@ -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(),
Expand All @@ -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:
Expand All @@ -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(),
Expand All @@ -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)
Expand All @@ -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):
Expand Down
5 changes: 3 additions & 2 deletions posthog/api/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down Expand Up @@ -421,7 +422,7 @@ def verify_email(self, request, **kwargs):

@action(
methods=["POST"],
detail=True,
detail=False,
permission_classes=[AllowAny],
throttle_classes=[UserEmailVerificationThrottle],
)
Expand Down
Loading