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: reimplement password reset #20966

Closed
wants to merge 18 commits into from
Closed
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ee/api/test/test_authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ def test_cannot_reset_password_with_enforced_sso(self):
EMAIL_HOST="localhost",
SITE_URL="https://my.posthog.net",
):
response = self.client.post("/api/reset/", {"email": "[email protected]"})
response = self.client.post("/api/users/@me/request_password_reset/", {"email": "[email protected]"})
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertEqual(
response.json(),
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
7 changes: 4 additions & 3 deletions frontend/src/scenes/authentication/passwordResetLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export const passwordResetLogic = kea<passwordResetLogicType>([
{
validateResetToken: async ({ uuid, token }: { uuid: string; token: string }) => {
try {
await api.get(`api/reset/${uuid}/?token=${token}`)
await api.create(`api/users/@me/validate_password_reset/`, { token, uuid })
return { success: true, token, uuid }
} catch (e: any) {
return { success: false, errorCode: e.code, errorDetail: e.detail }
Expand Down Expand Up @@ -67,7 +67,7 @@ export const passwordResetLogic = kea<passwordResetLogicType>([
breakpoint()

try {
await api.create('api/reset/', { email })
await api.create('api/users/@me/request_password_reset/', { email })
} catch (e: any) {
actions.setRequestPasswordResetManualErrors(e)
}
Expand Down Expand Up @@ -95,9 +95,10 @@ export const passwordResetLogic = kea<passwordResetLogicType>([
return
}
try {
await api.create(`api/reset/${values.validatedResetToken.uuid}/`, {
await api.create(`api/users/@me/reset_password/`, {
password,
token: values.validatedResetToken.token,
uuid: values.validatedResetToken.uuid,
})
lemonToast.success('Your password has been changed. Redirecting…')
await breakpoint(3000)
Expand Down
7 changes: 3 additions & 4 deletions posthog/admin/admins/user_admin.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
from django.utils.html import format_html

from django.contrib.auth.admin import UserAdmin as DjangoUserAdmin
from django.contrib.auth.forms import UserChangeForm as DjangoUserChangeForm
from django.contrib.auth.tokens import default_token_generator
from django.utils.html import format_html
from django.utils.translation import gettext_lazy as _

from posthog.admin.inlines.organization_member_inline import OrganizationMemberInline
from posthog.admin.inlines.totp_device_inline import TOTPDeviceInline
from posthog.api.password_reset import password_reset_token_generator
from posthog.models import User


Expand All @@ -18,7 +17,7 @@ def __init__(self, *args, **kwargs):
# we have a link to the password reset page which the _user_ can use themselves.
# This way if some user needs to reset their password and there's a problem with receiving the reset link email,
# an admin can provide that reset link manually – much better than sending a new password in plain text.
password_reset_token = default_token_generator.make_token(self.instance)
password_reset_token = password_reset_token_generator.make_token(self.instance)
self.fields["password"].help_text = (
"Raw passwords are not stored, so there is no way to see this user’s password, but you can send them "
f'<a target="_blank" href="/reset/{self.instance.uuid}/{password_reset_token}">this password reset link</a> '
Expand Down
12 changes: 9 additions & 3 deletions posthog/api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,14 @@
from posthog.api.routing import DefaultRouterPlusPlus
from posthog.batch_exports import http as batch_exports
from posthog.settings import EE_AVAILABLE
from posthog.warehouse.api import external_data_source, saved_query, table, view_link, external_data_schema
from posthog.warehouse.api import (
external_data_schema,
external_data_source,
saved_query,
table,
view_link,
)

from ..session_recordings.session_recording_api import SessionRecordingViewSet
from . import (
activity_log,
Expand Down Expand Up @@ -33,8 +40,8 @@
plugin_log_entry,
property_definition,
query,
search,
scheduled_change,
search,
sharing,
survey,
tagged_item,
Expand Down Expand Up @@ -296,7 +303,6 @@ def api_not_found(request):
router.register(r"login", authentication.LoginViewSet, "login")
router.register(r"login/token", authentication.TwoFactorViewSet)
router.register(r"login/precheck", authentication.LoginPrecheckViewSet)
router.register(r"reset", authentication.PasswordResetViewSet, "password_reset")
router.register(r"users", user.UserViewSet)
router.register(r"personal_api_keys", personal_api_key.PersonalAPIKeyViewSet, "personal_api_keys")
router.register(r"instance_status", instance_status.InstanceStatusViewSet, "instance_status")
Expand Down
156 changes: 1 addition & 155 deletions posthog/api/authentication.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,10 @@
import datetime
import time
from typing import Any, Dict, Optional, cast
from uuid import uuid4

from django.conf import settings
from django.contrib.auth import authenticate, login
from django.contrib.auth import views as auth_views
from django.contrib.auth.password_validation import validate_password
from django.contrib.auth.tokens import (
PasswordResetTokenGenerator as DefaultPasswordResetTokenGenerator,
)
from django.core.exceptions import ValidationError
from django.core.signing import BadSignature
from django.db import transaction
from django.http import HttpRequest, HttpResponse, JsonResponse
Expand All @@ -23,7 +17,6 @@
from rest_framework.request import Request
from rest_framework.response import Response
from rest_framework.throttling import UserRateThrottle
from sentry_sdk import capture_exception
from social_django.views import auth
from two_factor.utils import default_device
from two_factor.views.core import REMEMBER_COOKIE_PREFIX
Expand All @@ -34,9 +27,8 @@

from posthog.api.email_verification import EmailVerifier
from posthog.email import is_email_available
from posthog.event_usage import report_user_logged_in, report_user_password_reset
from posthog.event_usage import report_user_logged_in
from posthog.models import OrganizationDomain, User
from posthog.tasks.email import send_password_reset
from posthog.utils import get_instance_available_sso_providers


Expand Down Expand Up @@ -250,149 +242,3 @@ class LoginPrecheckViewSet(NonCreatingViewSetMixin, viewsets.GenericViewSet):
queryset = User.objects.none()
serializer_class = LoginPrecheckSerializer
permission_classes = (permissions.AllowAny,)


class PasswordResetSerializer(serializers.Serializer):
email = serializers.EmailField(write_only=True)

def create(self, validated_data):
email = validated_data.pop("email")

# Check SSO enforcement (which happens at the domain level)
if OrganizationDomain.objects.get_sso_enforcement_for_email_address(email):
raise serializers.ValidationError(
"Password reset is disabled because SSO login is enforced for this domain.",
code="sso_enforced",
)

if not is_email_available():
raise serializers.ValidationError(
"Cannot reset passwords because email is not configured for your instance. Please contact your administrator.",
code="email_not_available",
)

try:
user = User.objects.filter(is_active=True).get(email=email)
except User.DoesNotExist:
user = None

if user:
user.requested_password_reset_at = datetime.datetime.now(datetime.timezone.utc)
user.save()
token = password_reset_token_generator.make_token(user)
send_password_reset(user.id, token)

return True


class PasswordResetCompleteSerializer(serializers.Serializer):
token = serializers.CharField(write_only=True)
password = serializers.CharField(write_only=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

try:
user = User.objects.filter(is_active=True).get(uuid=self.context["view"].kwargs["user_uuid"])
except User.DoesNotExist:
capture_exception(
Exception("User not found in password reset serializer"),
{"user_uuid": self.context["view"].kwargs["user_uuid"]},
)
raise serializers.ValidationError(
{"token": ["This reset token is invalid or has expired."]},
code="invalid_token",
)

if not password_reset_token_generator.check_token(user, validated_data["token"]):
capture_exception(
Exception("Invalid password reset token in serializer"),
{"user_uuid": user.uuid, "token": validated_data["token"]},
)
raise serializers.ValidationError(
{"token": ["This reset token is invalid or has expired."]},
code="invalid_token",
)
password = validated_data["password"]
try:
validate_password(password, user)
except ValidationError as e:
raise serializers.ValidationError({"password": e.messages})

user.set_password(password)
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


class PasswordResetViewSet(NonCreatingViewSetMixin, viewsets.GenericViewSet):
queryset = User.objects.none()
serializer_class = PasswordResetSerializer
permission_classes = (permissions.AllowAny,)
throttle_classes = [UserPasswordResetThrottle]
SUCCESS_STATUS_CODE = status.HTTP_204_NO_CONTENT


class PasswordResetCompleteViewSet(NonCreatingViewSetMixin, mixins.RetrieveModelMixin, viewsets.GenericViewSet):
queryset = User.objects.none()
serializer_class = PasswordResetCompleteSerializer
permission_classes = (permissions.AllowAny,)
SUCCESS_STATUS_CODE = status.HTTP_204_NO_CONTENT

def get_object(self):
token = self.request.query_params.get("token")
user_uuid = self.kwargs.get("user_uuid")

if not token:
raise serializers.ValidationError({"token": ["This field is required."]}, code="required")

# Special handling for E2E tests
if settings.E2E_TESTING and user_uuid == "e2e_test_user" and token == "e2e_test_token":
return {"success": True, "token": token}

try:
user = User.objects.filter(is_active=True).get(uuid=user_uuid)
except User.DoesNotExist:
capture_exception(
Exception("User not found in password reset viewset"), {"user_uuid": user_uuid, "token": token}
)
raise serializers.ValidationError(
{"token": ["This reset token is invalid or has expired."]},
code="invalid_token",
)

if not password_reset_token_generator.check_token(user, token):
capture_exception(
Exception("Invalid password reset token in viewset"), {"user_uuid": user_uuid, "token": token}
)
raise serializers.ValidationError(
{"token": ["This reset token is invalid or has expired."]},
code="invalid_token",
)

return {"success": True, "token": token}

def retrieve(self, request: Request, *args: Any, **kwargs: Any) -> Response:
response = super().retrieve(request, *args, **kwargs)
response.status_code = self.SUCCESS_STATUS_CODE
return response


class PasswordResetTokenGenerator(DefaultPasswordResetTokenGenerator):
def _make_hash_value(self, user, 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"{user.pk}{user.email}{usable_user.requested_password_reset_at}{timestamp}"


password_reset_token_generator = PasswordResetTokenGenerator()
47 changes: 47 additions & 0 deletions posthog/api/password_reset.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import datetime

import structlog
from django.contrib.auth.models import AbstractBaseUser
from django.contrib.auth.tokens import PasswordResetTokenGenerator
from rest_framework import exceptions
from sentry_sdk import capture_exception

from posthog.models.user import User
from posthog.tasks.email import send_password_reset

logger = structlog.get_logger(__name__)


class PHPasswordResetTokenGenerator(PasswordResetTokenGenerator):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I think this should be in a sub folder like the services folder to differentiate it from api routers

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)
logger.info(
f"Password reset token for {usable_user.email} requested at {usable_user.requested_password_reset_at}"
)
return f"{usable_user.pk}{usable_user.email}{usable_user.requested_password_reset_at}{timestamp}"


password_reset_token_generator = PHPasswordResetTokenGenerator()


class PasswordResetter:
@staticmethod
def create_token_and_send_reset_email(user: User) -> None:
user.requested_password_reset_at = datetime.datetime.now(datetime.timezone.utc)
user.save()
token = password_reset_token_generator.make_token(user)
logger.info(f"Password reset requested for {user.email}")

try:
send_password_reset(user.pk, token)
except Exception as e:
capture_exception(Exception(f"Verification email failed: {e}"))
raise exceptions.APIException(
detail="Could not send email verification email. Please try again by logging in with your email and password."
)

@staticmethod
def check_token(user: User, token: str) -> bool:
return password_reset_token_generator.check_token(user, token)
Loading
Loading