fix: don't automatically log user in after password reset or email confirmation (#38283)

This commit is contained in:
Tom Piccirello
2025-09-18 13:27:57 -07:00
committed by GitHub
parent fcbee4e72e
commit c59affef4a
7 changed files with 45 additions and 35 deletions

View File

@@ -98,13 +98,18 @@ export const passwordResetLogic = kea<passwordResetLogicType>([
return
}
try {
await api.create(`api/reset/${values.validatedResetToken.uuid}/`, {
const response = await api.create(`api/reset/${values.validatedResetToken.uuid}/`, {
password,
token: values.validatedResetToken.token,
})
lemonToast.success('Your password has been changed. Redirecting…')
await breakpoint(3000)
window.location.href = '/' // We need the refresh
const url = new URL('/login', window.location.origin)
if (response.email) {
url.searchParams.set('email', response.email)
}
window.location.href = url.href // We need the refresh
} catch (e: any) {
actions.setPasswordResetManualErrors({ password: e.detail })
throw e

View File

@@ -54,7 +54,20 @@ test.describe('Password Reset', () => {
await page.goto('/reset/e2e_test_user/e2e_test_token')
await page.fill('[data-attr="password"]', VALID_PASSWORD)
await page.fill('[data-attr="password-confirm"]', VALID_PASSWORD)
// Intercept the password reset complete request to check response
const responsePromise = page.waitForResponse(
(response) => response.url().includes('/api/reset/e2e_test_user') && response.request().method() === 'POST'
)
await page.click('button[type=submit]')
const response = await responsePromise
expect(response.status()).toBe(200)
const responseBody = await response.json()
expect(responseBody.success).toBe(true)
expect(responseBody.email).toBe('test@posthog.com')
await expect(page.locator('.Toastify__toast--success')).toBeVisible()
await expect(page).not.toHaveURL('/reset/e2e_test_user/e2e_test_token')
})

View File

@@ -352,10 +352,15 @@ class PasswordResetCompleteSerializer(serializers.Serializer):
token = serializers.CharField(write_only=True)
password = serializers.CharField(write_only=True)
def to_representation(self, instance):
if isinstance(instance, dict) and "email" in instance:
return {"success": True, "email": instance["email"]}
return {"success": True}
def create(self, validated_data):
# Special handling for E2E tests (note we don't actually change anything in the DB, just simulate the response)
if settings.E2E_TESTING and validated_data["token"] == "e2e_test_token":
return True
return {"email": "test@posthog.com"}
try:
user = User.objects.filter(is_active=True).get(uuid=self.context["view"].kwargs["user_uuid"])
@@ -388,9 +393,8 @@ class PasswordResetCompleteSerializer(serializers.Serializer):
user.requested_password_reset_at = None
user.save()
login(self.context["request"], user, backend="django.contrib.auth.backends.ModelBackend")
report_user_password_reset(user)
return True
return {"email": user.email}
class PasswordResetViewSet(NonCreatingViewSetMixin, viewsets.GenericViewSet):
@@ -405,7 +409,7 @@ class PasswordResetCompleteViewSet(NonCreatingViewSetMixin, mixins.RetrieveModel
queryset = User.objects.none()
serializer_class = PasswordResetCompleteSerializer
permission_classes = (permissions.AllowAny,)
SUCCESS_STATUS_CODE = status.HTTP_204_NO_CONTENT
SUCCESS_STATUS_CODE = status.HTTP_200_OK
def get_object(self):
token = self.request.query_params.get("token")

View File

@@ -28,7 +28,7 @@ class EmailVerificationTokenGenerator(PasswordResetTokenGenerator):
usable_user: User = User.objects.get(pk=user.pk)
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}"
return f"{usable_user.pk}{usable_user.email}{usable_user.is_email_verified}{usable_user.pending_email}{login_timestamp}{timestamp}"
email_verification_token_generator = EmailVerificationTokenGenerator()

View File

@@ -586,7 +586,7 @@ class TestPasswordResetAPI(APIBaseTest):
def test_can_validate_token(self):
token = password_reset_token_generator.make_token(self.user)
response = self.client.get(f"/api/reset/{self.user.uuid}/?token={token}")
self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(response.content.decode(), "")
self.assertEqual(response.headers["Content-Length"], "0")
@@ -639,13 +639,12 @@ class TestPasswordResetAPI(APIBaseTest):
self.user.save()
token = password_reset_token_generator.make_token(self.user)
response = self.client.post(f"/api/reset/{self.user.uuid}/", {"token": token, "password": VALID_TEST_PASSWORD})
self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)
self.assertEqual(response.content.decode(), "")
# assert the user gets logged in automatically
response = self.client.get("/api/users/@me/")
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(response.json()["email"], self.CONFIG_EMAIL)
self.assertEqual(response.content.decode(), f'{{"success":true,"email":"{self.user.email}"}}')
# assert the user DOES NOT get logged in automatically
response = self.client.get("/api/users/@me/")
self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED)
# check password was changed
self.user.refresh_from_db()
@@ -793,14 +792,14 @@ class TestPasswordResetAPI(APIBaseTest):
def test_e2e_test_special_handlers(self):
with self.settings(E2E_TESTING=True):
response = self.client.get("/api/reset/e2e_test_user/?token=e2e_test_token")
self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)
self.assertEqual(response.status_code, status.HTTP_200_OK)
with self.settings(E2E_TESTING=True):
response = self.client.post(
"/api/reset/e2e_test_user/",
{"token": "e2e_test_token", "password": "a12345678"},
)
self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)
self.assertEqual(response.status_code, status.HTTP_200_OK)
class TestPersonalAPIKeyAuthentication(APIBaseTest):

View File

@@ -1463,16 +1463,6 @@ class TestEmailVerificationAPI(APIBaseTest):
self.assertTrue(self.user.is_email_verified)
# assert events were captured
mock_capture.assert_any_call(
event="user logged in",
distinct_id=self.user.distinct_id,
properties={"social_provider": ""},
groups={
"instance": ANY,
"organization": str(self.team.organization_id),
"project": str(self.team.uuid),
},
)
mock_capture.assert_any_call(
event="user verified email",
distinct_id=self.user.distinct_id,
@@ -1486,7 +1476,7 @@ class TestEmailVerificationAPI(APIBaseTest):
"organization": str(self.team.organization_id),
},
)
self.assertEqual(mock_capture.call_count, 3)
self.assertEqual(mock_capture.call_count, 2)
def test_cant_verify_if_email_is_not_configured(self):
set_instance_setting("EMAIL_HOST", "")
@@ -1590,7 +1580,7 @@ class TestEmailVerificationAPI(APIBaseTest):
},
)
def test_email_verification_logs_in_user(self):
def test_email_verification_does_not_log_in_user(self):
token = email_verification_token_generator.make_token(self.user)
self.client.logout()
@@ -1601,7 +1591,7 @@ class TestEmailVerificationAPI(APIBaseTest):
# 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)
assert session_user_id is None
def test_email_verification_logs_in_correctuser(self):
other_token = email_verification_token_generator.make_token(self.other_user)
@@ -1611,7 +1601,8 @@ class TestEmailVerificationAPI(APIBaseTest):
# 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)
# user should still be logged out
assert session_user_id is None
def test_email_verification_does_not_apply_to_current_logged_in_user(self):
other_token = email_verification_token_generator.make_token(self.other_user)
@@ -1621,7 +1612,7 @@ class TestEmailVerificationAPI(APIBaseTest):
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 self.client.session.get("_auth_user_id") is None
assert not self.user.is_email_verified
assert self.other_user.is_email_verified

View File

@@ -9,7 +9,7 @@ from datetime import UTC, datetime, timedelta
from typing import Any, Optional, cast
from django.conf import settings
from django.contrib.auth import login, update_session_auth_hash
from django.contrib.auth import update_session_auth_hash
from django.contrib.auth.password_validation import validate_password
from django.core.exceptions import ValidationError
from django.http import HttpResponse, JsonResponse
@@ -52,7 +52,7 @@ from posthog.auth import (
)
from posthog.constants import PERMITTED_FORUM_DOMAINS
from posthog.email import is_email_available
from posthog.event_usage import report_user_logged_in, report_user_updated, report_user_verified_email
from posthog.event_usage import report_user_updated, report_user_verified_email
from posthog.middleware import get_impersonated_session_expires_at
from posthog.models import Dashboard, Team, User, UserScenePersonalisation
from posthog.models.organization import Organization
@@ -477,8 +477,6 @@ class UserViewSet(
user.save()
report_user_verified_email(user)
login(self.request, user, backend="django.contrib.auth.backends.ModelBackend")
report_user_logged_in(user)
return Response({"success": True, "token": token})
@action(