Skip to content

Commit

Permalink
Do not throttle IPs of legitimate users (mozilla#3416)
Browse files Browse the repository at this point in the history
Change IP throttling to never block legitimate users (identified by having at least 1 approved translation). For blocked IPs we now also log the username if the request is coming from an authenticated user. Log message is also changed a little so it's easier to search for all IPs with excessive requests (blocked and legitimate).
  • Loading branch information
mathjazz authored Oct 18, 2024
1 parent baf98ac commit c546a21
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 6 deletions.
19 changes: 16 additions & 3 deletions pontoon/base/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,10 @@ def _throttle(self, request):
return response

def __call__(self, request):
response = self.get_response(request)

if settings.THROTTLE_ENABLED is False:
return self.get_response(request)
return response

ip = get_ip(request)

Expand All @@ -119,9 +121,21 @@ def __call__(self, request):
if request_data:
request_count, first_request_time = request_data
if request_count >= self.max_count:
user = request.user

# Do not block IPs of legitimate users
if user.is_authenticated and user.has_approved_translations:
log.info(f"Not blocking IP {ip} of user {user.username}")
return response

# Block further requests for block_duration seconds
cache.set(blocked_key, True, self.block_duration)
log.error(f"Blocked IP {ip} for {self.block_duration} seconds")
username = (
user.username if user.is_authenticated else "unauthenticated user"
)
log.error(
f"Blocking IP {ip} of {username} for {self.block_duration} seconds"
)
return self._throttle(request)
else:
# Increment the request count and update cache
Expand All @@ -134,5 +148,4 @@ def __call__(self, request):
# Reset the count and timestamp if first request in the period
cache.set(observed_key, (1, now), self.observation_period)

response = self.get_response(request)
return response
11 changes: 8 additions & 3 deletions pontoon/base/models/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,10 +210,14 @@ def user_status(self, locale):

@property
def contributed_translations(self):
"""Filtered contributions provided by user."""
from pontoon.base.models.translation import Translation
"""Contributions provided by user."""
return self.translation_set.all()

return Translation.objects.filter(user=self)

@property
def has_approved_translations(self):
"""Return True if the user has approved translations."""
return self.translation_set.filter(approved=True).exists()


@property
Expand Down Expand Up @@ -441,6 +445,7 @@ def latest_action(self):
User.add_to_class("locale_role", user_locale_role)
User.add_to_class("status", user_status)
User.add_to_class("contributed_translations", contributed_translations)
User.add_to_class("has_approved_translations", has_approved_translations)
User.add_to_class("top_contributed_locale", top_contributed_locale)
User.add_to_class("can_translate", can_translate)
User.add_to_class("is_new_contributor", is_new_contributor)
Expand Down

0 comments on commit c546a21

Please sign in to comment.