Skip to content

Commit

Permalink
fix(password-reset): rate limit password reset emails (#2619)
Browse files Browse the repository at this point in the history
  • Loading branch information
gagantrivedi committed Aug 15, 2023
1 parent 4df1b80 commit db98743
Show file tree
Hide file tree
Showing 7 changed files with 154 additions and 1 deletion.
6 changes: 6 additions & 0 deletions api/app/settings/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -903,3 +903,9 @@
DOMAIN_OVERRIDE = env.str("FLAGSMITH_DOMAIN", "")
# Used when no Django site is specified.
DEFAULT_DOMAIN = "app.flagsmith.com"

# Define the cooldown duration, in seconds, for password reset emails
PASSWORD_RESET_EMAIL_COOLDOWN = env.int("PASSWORD_RESET_EMAIL_COOLDOWN", 60 * 60 * 24)

# Limit the count of password reset emails that can be dispatched within the `PASSWORD_RESET_EMAIL_COOLDOWN` timeframe.
MAX_PASSWORD_RESET_EMAILS = env.int("MAX_PASSWORD_RESET_EMAILS", 5)
25 changes: 25 additions & 0 deletions api/custom_auth/migrations/0001_initial.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Generated by Django 3.2.20 on 2023-08-15 05:15

from django.conf import settings
from django.db import migrations, models
import django.db.models.deletion


class Migration(migrations.Migration):

initial = True

dependencies = [
migrations.swappable_dependency(settings.AUTH_USER_MODEL),
]

operations = [
migrations.CreateModel(
name='UserPasswordResetRequest',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('requested_at', models.DateTimeField(auto_now_add=True)),
('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='password_reset_requests', to=settings.AUTH_USER_MODEL)),
],
),
]
Empty file.
10 changes: 10 additions & 0 deletions api/custom_auth/models.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
from django.db import models

from users.models import FFAdminUser


class UserPasswordResetRequest(models.Model):
user = models.ForeignKey(
FFAdminUser, related_name="password_reset_requests", on_delete=models.CASCADE
)
requested_at = models.DateTimeField(auto_now_add=True)
15 changes: 14 additions & 1 deletion api/custom_auth/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from drf_yasg.utils import swagger_auto_schema
from rest_framework import status
from rest_framework.authtoken.models import Token
from rest_framework.decorators import api_view, permission_classes
from rest_framework.decorators import action, api_view, permission_classes
from rest_framework.permissions import IsAuthenticated
from rest_framework.response import Response
from rest_framework.throttling import ScopedRateThrottle
Expand All @@ -16,6 +16,8 @@
from custom_auth.serializers import CustomUserDelete
from users.constants import DEFAULT_DELETE_ORPHAN_ORGANISATIONS_VALUE

from .models import UserPasswordResetRequest


class CustomAuthTokenLoginOrRequestMFACode(AuthTokenLoginOrRequestMFACode):
"""
Expand Down Expand Up @@ -68,3 +70,14 @@ def perform_destroy(self, instance):
DEFAULT_DELETE_ORPHAN_ORGANISATIONS_VALUE,
)
)

@action(["post"], detail=False)
def reset_password(self, request, *args, **kwargs):
serializer = self.get_serializer(data=request.data)
serializer.is_valid(raise_exception=True)
user = serializer.get_user()
if user and user.can_send_password_reset_email():
super().reset_password(request, *args, **kwargs)
UserPasswordResetRequest.objects.create(user=user)

return Response(status=status.HTTP_204_NO_CONTENT)
85 changes: 85 additions & 0 deletions api/tests/unit/users/test_unit_users_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@

import pytest
from django.conf import settings
from django.contrib.auth.tokens import default_token_generator
from django.core import mail
from django.urls import reverse
from djoser import utils
from djoser.email import PasswordResetEmail
from rest_framework import status
from rest_framework.test import APIClient

Expand Down Expand Up @@ -117,3 +121,84 @@ def test_change_email_address_api(mocker):
assert len(args) == 0
assert len(kwargs) == 1
assert kwargs["args"] == (first_name, settings.DEFAULT_FROM_EMAIL, old_email)


@pytest.mark.django_db
def test_send_reset_password_emails_rate_limit(settings, client, test_user):
# Given
settings.MAX_PASSWORD_RESET_EMAILS = 2
settings.PASSWORD_RESET_EMAIL_COOLDOWN = 60

url = reverse("api-v1:custom_auth:ffadminuser-reset-password")
data = {"email": test_user.email}

# When
for _ in range(5):
response = client.post(
url, data=json.dumps(data), content_type="application/json"
)
assert response.status_code == status.HTTP_204_NO_CONTENT

# Then - we should only have two emails
assert len(mail.outbox) == 2
isinstance(mail.outbox[0], PasswordResetEmail)
isinstance(mail.outbox[1], PasswordResetEmail)

# clear the outbox
mail.outbox.clear()

# Next, let's reduce the cooldown to 1 second and try again
settings.PASSWORD_RESET_EMAIL_COOLDOWN = 0.001
response = client.post(url, data=json.dumps(data), content_type="application/json")

# Then - we should receive another email
assert response.status_code == status.HTTP_204_NO_CONTENT

assert len(mail.outbox) == 1
isinstance(mail.outbox[0], PasswordResetEmail)


@pytest.mark.django_db
def test_send_reset_password_emails_rate_limit_resets_after_password_reset(
settings, client, test_user
):
# Given
settings.MAX_PASSWORD_RESET_EMAILS = 2
settings.PASSWORD_RESET_EMAIL_COOLDOWN = 60 * 60 * 24

url = reverse("api-v1:custom_auth:ffadminuser-reset-password")
data = {"email": test_user.email}

# First, let's hit the limit of emails we can send
for _ in range(5):
response = client.post(
url, data=json.dumps(data), content_type="application/json"
)
assert response.status_code == status.HTTP_204_NO_CONTENT

# Then - we should only have two emails
assert len(mail.outbox) == 2
mail.outbox.clear()

# Next, let's reset the password
reset_password_data = {
"new_password": "new_password",
"re_new_password": "new_password",
"uid": utils.encode_uid(test_user.pk),
"token": default_token_generator.make_token(test_user),
}
reset_password_confirm_url = reverse(
"api-v1:custom_auth:ffadminuser-reset-password-confirm"
)
response = client.post(
reset_password_confirm_url,
data=json.dumps(reset_password_data),
content_type="application/json",
)
assert response.status_code == status.HTTP_204_NO_CONTENT

# Finally, let's try to send another email
client.post(url, data=json.dumps(data), content_type="application/json")

# Then - we should receive another email
assert len(mail.outbox) == 1
14 changes: 14 additions & 0 deletions api/users/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from django.core.mail import send_mail
from django.db import models
from django.db.models import Count, QuerySet
from django.utils import timezone
from django.utils.translation import gettext_lazy as _
from django_lifecycle import AFTER_CREATE, LifecycleModel, hook

Expand Down Expand Up @@ -135,6 +136,10 @@ def delete(
self.delete_orphan_organisations()
super().delete()

def set_password(self, raw_password):
super().set_password(raw_password)
self.password_reset_requests.all().delete()

@property
def auth_type(self):
if self.google_user_id:
Expand All @@ -158,6 +163,15 @@ def get_full_name(self):
return None
return " ".join([self.first_name, self.last_name]).strip()

def can_send_password_reset_email(self) -> bool:
limit = timezone.now() - timezone.timedelta(
seconds=settings.PASSWORD_RESET_EMAIL_COOLDOWN
)
return (
self.password_reset_requests.filter(requested_at__gte=limit).count()
< settings.MAX_PASSWORD_RESET_EMAILS
)

def join_organisation_from_invite_email(self, invite_email: "Invite"):
if invite_email.email.lower() != self.email.lower():
raise InvalidInviteError("Registered email does not match invited email")
Expand Down

3 comments on commit db98743

@vercel
Copy link

@vercel vercel bot commented on db98743 Aug 15, 2023

Choose a reason for hiding this comment

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

@vercel
Copy link

@vercel vercel bot commented on db98743 Aug 15, 2023

Choose a reason for hiding this comment

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

Successfully deployed to the following URLs:

docs – ./docs

docs-git-main-flagsmith.vercel.app
docs-flagsmith.vercel.app
docs.flagsmith.com
docs.bullet-train.io

@vercel
Copy link

@vercel vercel bot commented on db98743 Aug 15, 2023

Choose a reason for hiding this comment

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

Please sign in to comment.