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.

Sign up for GitHub

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

Add referrer based throttle scope #3486

Merged
merged 4 commits into from
Dec 19, 2023
Merged

Add referrer based throttle scope #3486

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
44deca4
Remove ip-whitelist throttle exemption
sarayourfriend Dec 7, 2023
c2880cc
Add additional throttle scope for openverse.org referrer
sarayourfriend Dec 7, 2023
214f029
Refactor throttle unit tests to exercise full throttle lifecycle and …
sarayourfriend Dec 7, 2023
297d74b
Use a set of rate limit models instead of mixed type
sarayourfriend Dec 11, 2023
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
80 changes: 52 additions & 28 deletions api/api/utils/throttle.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
import abc
import logging

from rest_framework.throttling import SimpleRateThrottle

from django_redis import get_redis_connection
from rest_framework.throttling import SimpleRateThrottle as BaseSimpleRateThrottle

from api.utils.oauth2_helper import get_token_info


parent_logger = logging.getLogger(__name__)


class SimpleRateThrottleHeader(SimpleRateThrottle, metaclass=abc.ABCMeta):
class SimpleRateThrottle(BaseSimpleRateThrottle, metaclass=abc.ABCMeta):
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I renamed this because it no longer only handled header-related enhancements.

"""
Extends the ``SimpleRateThrottle`` class to provide additional functionality such as
rate-limit headers in the response.
Expand All @@ -38,39 +36,53 @@ def headers(self):
else:
return {}

def has_valid_token(self, request):
if not request.auth:
return False

token_info = get_token_info(str(request.auth))
return token_info and token_info.valid

class AbstractAnonRateThrottle(SimpleRateThrottleHeader, metaclass=abc.ABCMeta):
def get_cache_key(self, request, view):
ident = self.get_ident(request)
return self.cache_format % {
"scope": self.scope,
"ident": ident,
}


class AbstractAnonRateThrottle(SimpleRateThrottle, metaclass=abc.ABCMeta):
"""
Limits the rate of API calls that may be made by a anonymous users.

The IP address of the request will be used as the unique cache key.
"""

logger = parent_logger.getChild("AnonRateThrottle")
def get_cache_key(self, request, view):
# Do not apply this throttle to requests with valid tokens
if self.has_valid_token(request):
return None

if request.headers.get("referrer") == "openverse.org":
# Use `ov_referrer` throttles instead
return None

return super().get_cache_key(request, view)


class AbstractOpenverseReferrerRateThrottle(SimpleRateThrottle, metaclass=abc.ABCMeta):
"""Use a different limit for requests that appear to come from Openverse.org."""

def get_cache_key(self, request, view):
logger = self.logger.getChild("get_cache_key")
# Do not apply anonymous throttle to request with valid tokens.
if request.auth:
token_info = get_token_info(str(request.auth))
if token_info and token_info.valid:
return None
# Do not apply this throttle to requests with valid tokens
if self.has_valid_token(request):
return None

ident = self.get_ident(request)
redis = get_redis_connection("default", write=False)
if redis.sismember("ip-whitelist", ident):
logger.info(f"bypassing rate limiting for ident={ident}")
"""
Exempt internal IP addresses. Exists as a legacy holdover and usages of this
should be replaced with the exempt API key as it is easier to manage via
Django admin and doesn't require leaky permissions in our production infra.
"""
if request.headers.get("referrer") != "openverse.org":
# Use regular anon throttles instead
return None

return self.cache_format % {
"scope": self.scope,
"ident": ident,
}
return super().get_cache_key(request, view)


class BurstRateThrottle(AbstractAnonRateThrottle):
Expand All @@ -89,6 +101,18 @@ class AnonThumbnailRateThrottle(AbstractAnonRateThrottle):
scope = "anon_thumbnail"


class OpenverseReferrerBurstRateThrottle(AbstractOpenverseReferrerRateThrottle):
scope = "ov_referrer_burst"


class OpenverseReferrerSustainedRateThrottle(AbstractOpenverseReferrerRateThrottle):
scope = "ov_referrer_sustained"


class OpenverseReferrerAnonThumbnailRateThrottle(AbstractOpenverseReferrerRateThrottle):
scope = "ov_referrer_thumbnail"


class TenPerDay(AbstractAnonRateThrottle):
rate = "10/day"

Expand All @@ -97,7 +121,7 @@ class OnePerSecond(AbstractAnonRateThrottle):
rate = "1/second"


class AbstractOAuth2IdRateThrottle(SimpleRateThrottleHeader, metaclass=abc.ABCMeta):
class AbstractOAuth2IdRateThrottle(SimpleRateThrottle, metaclass=abc.ABCMeta):
"""
Ties a particular throttling scope from ``settings.py`` to a rate limit model.

Expand All @@ -116,14 +140,14 @@ def get_cache_key(self, request, view):
if not (token_info and token_info.valid):
return None

if token_info.rate_limit_model != self.applies_to_rate_limit_model:
if token_info.rate_limit_model not in self.applies_to_rate_limit_model:
sarayourfriend marked this conversation as resolved.
Show resolved Hide resolved
return None

return self.cache_format % {"scope": self.scope, "ident": token_info.client_id}


class OAuth2IdThumbnailRateThrottle(AbstractOAuth2IdRateThrottle):
applies_to_rate_limit_model = "standard"
applies_to_rate_limit_model = ["standard", "enhanced"]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

"enhanced" previously had no thumbnail rate applied to it. I don't think that's the correct behaviour, but I could be wrong. This change makes enhanced tokens have the same thumbnail rate limit as standard oauth. I think that's right because we have no separate thumbnail rate limit configured for enhanced tokens.

Exempt tokens, I assume, should be exempted from all rate limiting, so there's no change for that. That already worked.

scope = "oauth2_client_credentials_thumbnail"


Expand Down
12 changes: 10 additions & 2 deletions api/api/views/media_views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@
from api.utils import image_proxy
from api.utils.pagination import StandardPagination
from api.utils.search_context import SearchContext
from api.utils.throttle import AnonThumbnailRateThrottle, OAuth2IdThumbnailRateThrottle
from api.utils.throttle import (
AnonThumbnailRateThrottle,
OAuth2IdThumbnailRateThrottle,
OpenverseReferrerAnonThumbnailRateThrottle,
)


logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -300,7 +304,11 @@ async def get_image_proxy_media_info(self) -> image_proxy.MediaInfo:
url_path="thumb",
url_name="thumb",
serializer_class=media_serializers.MediaThumbnailRequestSerializer,
throttle_classes=[AnonThumbnailRateThrottle, OAuth2IdThumbnailRateThrottle],
throttle_classes=[
AnonThumbnailRateThrottle,
OpenverseReferrerAnonThumbnailRateThrottle,
OAuth2IdThumbnailRateThrottle,
],
)

async def thumbnail(self, request, *_, **__):
Expand Down
65 changes: 41 additions & 24 deletions api/conf/settings/rest_framework.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,45 @@
THROTTLE_OAUTH2_THUMBS = config("THROTTLE_OAUTH2_THUMBS", default="500/minute")
THROTTLE_ANON_HEALTHCHECK = config("THROTTLE_ANON_HEALTHCHECK", default="3/minute")

THROTTLE_OV_REFERRER_BURST = config(
"THROTTLE_OV_REFERRER_BURST", default=THROTTLE_ANON_BURST
)
THROTTLE_OV_REFERRER_SUSTAINED = config(
"THROTTLE_OV_REFERRER_SUSTAINED", default=THROTTLE_ANON_SUSTAINED
)
THROTTLE_OV_REFERRER_THUMBS = config(
"THROTTLE_OV_REFERRER_THUMBS", default=THROTTLE_ANON_THUMBS
)

DEFAULT_THROTTLE_RATES = {
"anon_burst": THROTTLE_ANON_BURST,
"anon_sustained": THROTTLE_ANON_SUSTAINED,
"anon_healthcheck": THROTTLE_ANON_HEALTHCHECK,
"anon_thumbnail": THROTTLE_ANON_THUMBS,
"ov_referrer_burst": THROTTLE_OV_REFERRER_BURST,
"ov_referrer_sustained": THROTTLE_OV_REFERRER_SUSTAINED,
"ov_referrer_thumbnail": THROTTLE_OV_REFERRER_THUMBS,
"oauth2_client_credentials_thumbnail": THROTTLE_OAUTH2_THUMBS,
"oauth2_client_credentials_sustained": "10000/day",
"oauth2_client_credentials_burst": "100/min",
"enhanced_oauth2_client_credentials_sustained": "20000/day",
"enhanced_oauth2_client_credentials_burst": "200/min",
# ``None`` completely by-passes the rate limiting
"exempt_oauth2_client_credentials": None,
}

DEFAULT_THROTTLE_CLASSES = (
"api.utils.throttle.BurstRateThrottle",
"api.utils.throttle.SustainedRateThrottle",
"api.utils.throttle.OpenverseReferrerBurstRateThrottle",
"api.utils.throttle.OpenverseReferrerSustainedRateThrottle",
"api.utils.throttle.OAuth2IdBurstRateThrottle",
"api.utils.throttle.OAuth2IdSustainedRateThrottle",
"api.utils.throttle.EnhancedOAuth2IdBurstRateThrottle",
"api.utils.throttle.EnhancedOAuth2IdSustainedRateThrottle",
"api.utils.throttle.ExemptOAuth2IdRateThrottle",
)
Comment on lines +26 to +53
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I moved these so that they are easier to reference in the tests when we need to enable the throttling for a scoped set of tests.


REST_FRAMEWORK = {
"DEFAULT_AUTHENTICATION_CLASSES": (
"oauth2_provider.contrib.rest_framework.OAuth2Authentication",
Expand All @@ -22,30 +61,8 @@
"rest_framework.renderers.JSONRenderer",
"api.utils.drf_renderer.BrowsableAPIRendererWithoutForms",
),
"DEFAULT_THROTTLE_CLASSES": (
"api.utils.throttle.BurstRateThrottle",
"api.utils.throttle.SustainedRateThrottle",
"api.utils.throttle.AnonThumbnailRateThrottle",
"api.utils.throttle.OAuth2IdThumbnailRateThrottle",
"api.utils.throttle.OAuth2IdSustainedRateThrottle",
"api.utils.throttle.OAuth2IdBurstRateThrottle",
"api.utils.throttle.EnhancedOAuth2IdSustainedRateThrottle",
"api.utils.throttle.EnhancedOAuth2IdBurstRateThrottle",
"api.utils.throttle.ExemptOAuth2IdRateThrottle",
),
"DEFAULT_THROTTLE_RATES": {
"anon_burst": THROTTLE_ANON_BURST,
"anon_sustained": THROTTLE_ANON_SUSTAINED,
"anon_healthcheck": THROTTLE_ANON_HEALTHCHECK,
"anon_thumbnail": THROTTLE_ANON_THUMBS,
"oauth2_client_credentials_thumbnail": THROTTLE_OAUTH2_THUMBS,
"oauth2_client_credentials_sustained": "10000/day",
"oauth2_client_credentials_burst": "100/min",
"enhanced_oauth2_client_credentials_sustained": "20000/day",
"enhanced_oauth2_client_credentials_burst": "200/min",
# ``None`` completely by-passes the rate limiting
"exempt_oauth2_client_credentials": None,
},
"DEFAULT_THROTTLE_CLASSES": DEFAULT_THROTTLE_CLASSES,
"DEFAULT_THROTTLE_RATES": DEFAULT_THROTTLE_RATES.copy(),
"EXCEPTION_HANDLER": "api.utils.exceptions.exception_handler",
"DEFAULT_SCHEMA_CLASS": "api.docs.base_docs.MediaSchema",
# https://www.django-rest-framework.org/api-guide/throttling/#how-clients-are-identified
Expand Down
Loading
Loading