From d28bf65d0047456d769f87fafd82fb887847a346 Mon Sep 17 00:00:00 2001 From: Eliot Berriot <contact@eliotberriot.com> Date: Tue, 17 Sep 2019 11:23:59 +0200 Subject: [PATCH] See #261: Added a rate-limiting (throttling system) to limit the number of requests handled per user/IP --- api/config/api_urls.py | 3 +- api/config/settings/common.py | 144 +++++++++ api/config/urls.py | 3 +- api/funkwhale_api/common/middleware.py | 66 +++++ api/funkwhale_api/common/throttling.py | 151 ++++++++++ api/funkwhale_api/common/views.py | 19 ++ api/funkwhale_api/moderation/views.py | 6 + api/funkwhale_api/users/jwt_views.py | 15 + api/funkwhale_api/users/oauth/views.py | 20 +- api/funkwhale_api/users/rest_auth_urls.py | 34 ++- api/funkwhale_api/users/views.py | 23 +- api/tests/common/test_middleware.py | 40 +++ api/tests/common/test_throttling.py | 337 ++++++++++++++++++++++ api/tests/common/test_views.py | 18 ++ api/tests/conftest.py | 8 + changes/changelog.d/261.feature | 1 + changes/notes.rst | 12 + docs/swagger.yml | 159 +++++++++- front/src/components/auth/Login.vue | 4 +- front/src/main.js | 33 ++- 20 files changed, 1077 insertions(+), 19 deletions(-) create mode 100644 api/funkwhale_api/common/throttling.py create mode 100644 api/funkwhale_api/users/jwt_views.py create mode 100644 api/tests/common/test_throttling.py create mode 100644 changes/changelog.d/261.feature diff --git a/api/config/api_urls.py b/api/config/api_urls.py index da1981585..cdcc74391 100644 --- a/api/config/api_urls.py +++ b/api/config/api_urls.py @@ -2,7 +2,6 @@ from django.conf.urls import include, url from dynamic_preferences.api.viewsets import GlobalPreferencesViewSet from rest_framework import routers from rest_framework.urlpatterns import format_suffix_patterns -from rest_framework_jwt import views as jwt_views from funkwhale_api.activity import views as activity_views from funkwhale_api.common import views as common_views @@ -11,6 +10,7 @@ from funkwhale_api.music import views from funkwhale_api.playlists import views as playlists_views from funkwhale_api.subsonic.views import SubsonicViewSet from funkwhale_api.tags import views as tags_views +from funkwhale_api.users import jwt_views router = common_routers.OptionalSlashRouter() router.register(r"settings", GlobalPreferencesViewSet, basename="settings") @@ -83,6 +83,7 @@ v1_patterns += [ ), url(r"^token/?$", jwt_views.obtain_jwt_token, name="token"), url(r"^token/refresh/?$", jwt_views.refresh_jwt_token, name="token_refresh"), + url(r"^rate-limit/?$", common_views.RateLimitView.as_view(), name="rate-limit"), ] urlpatterns = [ diff --git a/api/config/settings/common.py b/api/config/settings/common.py index 86950d726..c11d847e4 100644 --- a/api/config/settings/common.py +++ b/api/config/settings/common.py @@ -232,6 +232,7 @@ MIDDLEWARE = ( "django.contrib.auth.middleware.AuthenticationMiddleware", "django.contrib.messages.middleware.MessageMiddleware", "funkwhale_api.users.middleware.RecordActivityMiddleware", + "funkwhale_api.common.middleware.ThrottleStatusMiddleware", ) # DEBUG @@ -615,7 +616,150 @@ REST_FRAMEWORK = { "django_filters.rest_framework.DjangoFilterBackend", ), "DEFAULT_RENDERER_CLASSES": ("rest_framework.renderers.JSONRenderer",), + "NUM_PROXIES": env.int("NUM_PROXIES", default=1), } +THROTTLING_ENABLED = env.bool("THROTTLING_ENABLED", default=True) +if THROTTLING_ENABLED: + REST_FRAMEWORK["DEFAULT_THROTTLE_CLASSES"] = env.list( + "THROTTLE_CLASSES", + default=["funkwhale_api.common.throttling.FunkwhaleThrottle"], + ) + +THROTTLING_SCOPES = { + "*": {"anonymous": "anonymous-wildcard", "authenticated": "authenticated-wildcard"}, + "create": { + "authenticated": "authenticated-create", + "anonymous": "anonymous-create", + }, + "list": {"authenticated": "authenticated-list", "anonymous": "anonymous-list"}, + "retrieve": { + "authenticated": "authenticated-retrieve", + "anonymous": "anonymous-retrieve", + }, + "destroy": { + "authenticated": "authenticated-destroy", + "anonymous": "anonymous-destroy", + }, + "update": { + "authenticated": "authenticated-update", + "anonymous": "anonymous-update", + }, + "partial_update": { + "authenticated": "authenticated-update", + "anonymous": "anonymous-update", + }, +} + +THROTTLING_USER_RATES = env.dict("THROTTLING_RATES", default={}) + +THROTTLING_RATES = { + "anonymous-wildcard": { + "rate": THROTTLING_USER_RATES.get("anonymous-wildcard", "1000/h"), + "description": "Anonymous requests not covered by other limits", + }, + "authenticated-wildcard": { + "rate": THROTTLING_USER_RATES.get("authenticated-wildcard", "2000/h"), + "description": "Authenticated requests not covered by other limits", + }, + "authenticated-create": { + "rate": THROTTLING_USER_RATES.get("authenticated-create", "1000/hour"), + "description": "Authenticated POST requests", + }, + "anonymous-create": { + "rate": THROTTLING_USER_RATES.get("anonymous-create", "1000/day"), + "description": "Anonymous POST requests", + }, + "authenticated-list": { + "rate": THROTTLING_USER_RATES.get("authenticated-list", "10000/hour"), + "description": "Authenticated GET requests on resource lists", + }, + "anonymous-list": { + "rate": THROTTLING_USER_RATES.get("anonymous-list", "10000/day"), + "description": "Anonymous GET requests on resource lists", + }, + "authenticated-retrieve": { + "rate": THROTTLING_USER_RATES.get("authenticated-retrieve", "10000/hour"), + "description": "Authenticated GET requests on resource detail", + }, + "anonymous-retrieve": { + "rate": THROTTLING_USER_RATES.get("anonymous-retrieve", "10000/day"), + "description": "Anonymous GET requests on resource detail", + }, + "authenticated-destroy": { + "rate": THROTTLING_USER_RATES.get("authenticated-destroy", "500/hour"), + "description": "Authenticated DELETE requests on resource detail", + }, + "anonymous-destroy": { + "rate": THROTTLING_USER_RATES.get("anonymous-destroy", "1000/day"), + "description": "Anonymous DELETE requests on resource detail", + }, + "authenticated-update": { + "rate": THROTTLING_USER_RATES.get("authenticated-update", "1000/hour"), + "description": "Authenticated PATCH and PUT requests on resource detail", + }, + "anonymous-update": { + "rate": THROTTLING_USER_RATES.get("anonymous-update", "1000/day"), + "description": "Anonymous PATCH and PUT requests on resource detail", + }, + # potentially spammy / dangerous endpoints + "authenticated-reports": { + "rate": THROTTLING_USER_RATES.get("authenticated-reports", "100/day"), + "description": "Authenticated report submission", + }, + "anonymous-reports": { + "rate": THROTTLING_USER_RATES.get("anonymous-reports", "10/day"), + "description": "Anonymous report submission", + }, + "authenticated-oauth-app": { + "rate": THROTTLING_USER_RATES.get("authenticated-oauth-app", "10/hour"), + "description": "Authenticated OAuth app creation", + }, + "anonymous-oauth-app": { + "rate": THROTTLING_USER_RATES.get("anonymous-oauth-app", "10/day"), + "description": "Anonymous OAuth app creation", + }, + "oauth-authorize": { + "rate": THROTTLING_USER_RATES.get("oauth-authorize", "100/hour"), + "description": "OAuth app authorization", + }, + "oauth-token": { + "rate": THROTTLING_USER_RATES.get("oauth-token", "100/hour"), + "description": "OAuth token creation", + }, + "oauth-revoke-token": { + "rate": THROTTLING_USER_RATES.get("oauth-revoke-token", "100/hour"), + "description": "OAuth token deletion", + }, + "jwt-login": { + "rate": THROTTLING_USER_RATES.get("jwt-login", "30/hour"), + "description": "JWT token creation", + }, + "jwt-refresh": { + "rate": THROTTLING_USER_RATES.get("jwt-refresh", "30/hour"), + "description": "JWT token refresh", + }, + "signup": { + "rate": THROTTLING_USER_RATES.get("signup", "10/day"), + "description": "Account creation", + }, + "verify-email": { + "rate": THROTTLING_USER_RATES.get("verify-email", "20/h"), + "description": "Email address confirmation", + }, + "password-change": { + "rate": THROTTLING_USER_RATES.get("password-change", "20/h"), + "description": "Password change (when authenticated)", + }, + "password-reset": { + "rate": THROTTLING_USER_RATES.get("password-reset", "20/h"), + "description": "Password reset request", + }, + "password-reset-confirm": { + "rate": THROTTLING_USER_RATES.get("password-reset-confirm", "20/h"), + "description": "Password reset confirmation", + }, +} + BROWSABLE_API_ENABLED = env.bool("BROWSABLE_API_ENABLED", default=False) if BROWSABLE_API_ENABLED: diff --git a/api/config/urls.py b/api/config/urls.py index 99fc32f1f..015047f67 100644 --- a/api/config/urls.py +++ b/api/config/urls.py @@ -19,8 +19,7 @@ urlpatterns = [ ("funkwhale_api.federation.urls", "federation"), namespace="federation" ), ), - url(r"^api/v1/auth/", include("rest_auth.urls")), - url(r"^api/v1/auth/registration/", include("funkwhale_api.users.rest_auth_urls")), + url(r"^api/v1/auth/", include("funkwhale_api.users.rest_auth_urls")), url(r"^accounts/", include("allauth.urls")), # Your stuff: custom urls includes go here ] diff --git a/api/funkwhale_api/common/middleware.py b/api/funkwhale_api/common/middleware.py index c7bcf4b19..6866148b9 100644 --- a/api/funkwhale_api/common/middleware.py +++ b/api/funkwhale_api/common/middleware.py @@ -1,13 +1,16 @@ import html import requests +import time import xml.sax.saxutils from django import http from django.conf import settings from django.core.cache import caches from django import urls +from rest_framework import views from . import preferences +from . import throttling from . import utils EXCLUDED_PATHS = ["/api", "/federation", "/.well-known"] @@ -176,3 +179,66 @@ class DevHttpsMiddleware: lambda: request.__class__.get_host(request).replace(":80", ":443"), ) return self.get_response(request) + + +def monkey_patch_rest_initialize_request(): + """ + Rest framework use it's own APIRequest, meaning we can't easily + access our throttling info in the middleware. So me monkey patch the + `initialize_request` method from rest_framework to keep a link between both requests + """ + original = views.APIView.initialize_request + + def replacement(self, request, *args, **kwargs): + r = original(self, request, *args, **kwargs) + setattr(request, "_api_request", r) + return r + + setattr(views.APIView, "initialize_request", replacement) + + +monkey_patch_rest_initialize_request() + + +class ThrottleStatusMiddleware: + """ + Include useful information regarding throttling in API responses to + ensure clients can adapt. + """ + + def __init__(self, get_response): + self.get_response = get_response + + def __call__(self, request): + try: + response = self.get_response(request) + except throttling.TooManyRequests: + # manual throttling in non rest_framework view, we have to return + # the proper response ourselves + response = http.HttpResponse(status=429) + request_to_check = request + try: + request_to_check = request._api_request + except AttributeError: + pass + throttle_status = getattr(request_to_check, "_throttle_status", None) + if throttle_status: + response["X-RateLimit-Limit"] = str(throttle_status["num_requests"]) + response["X-RateLimit-Scope"] = str(throttle_status["scope"]) + response["X-RateLimit-Remaining"] = throttle_status["num_requests"] - len( + throttle_status["history"] + ) + response["X-RateLimit-Duration"] = str(throttle_status["duration"]) + if throttle_status["history"]: + now = int(time.time()) + # At this point, the client can send additional requests + oldtest_request = throttle_status["history"][-1] + remaining = throttle_status["duration"] - (now - int(oldtest_request)) + response["Retry-After"] = str(remaining) + # At this point, all Rate Limit is reset to 0 + latest_request = throttle_status["history"][0] + remaining = throttle_status["duration"] - (now - int(latest_request)) + response["X-RateLimit-Reset"] = str(now + remaining) + response["X-RateLimit-ResetSeconds"] = str(remaining) + + return response diff --git a/api/funkwhale_api/common/throttling.py b/api/funkwhale_api/common/throttling.py new file mode 100644 index 000000000..fe4a5d934 --- /dev/null +++ b/api/funkwhale_api/common/throttling.py @@ -0,0 +1,151 @@ +import collections + +from django.core.cache import cache +from rest_framework import throttling as rest_throttling + +from django.conf import settings + + +def get_ident(request): + if hasattr(request, "user") and request.user.is_authenticated: + return {"type": "authenticated", "id": request.user.pk} + ident = rest_throttling.BaseThrottle().get_ident(request) + + return {"type": "anonymous", "id": ident} + + +def get_cache_key(scope, ident): + parts = ["throttling", scope, ident["type"], str(ident["id"])] + return ":".join(parts) + + +def get_scope_for_action_and_ident_type(action, ident_type, view_conf={}): + config = collections.ChainMap(view_conf, settings.THROTTLING_SCOPES) + + try: + action_config = config[action] + except KeyError: + action_config = config.get("*", {}) + + try: + return action_config[ident_type] + except KeyError: + return + + +def get_status(ident, now): + data = [] + throttle = FunkwhaleThrottle() + for key in sorted(settings.THROTTLING_RATES.keys()): + conf = settings.THROTTLING_RATES[key] + row_data = {"id": key, "rate": conf["rate"], "description": conf["description"]} + if conf["rate"]: + num_requests, duration = throttle.parse_rate(conf["rate"]) + history = cache.get(get_cache_key(key, ident)) or [] + + relevant_history = [h for h in history if h > now - duration] + row_data["limit"] = num_requests + row_data["duration"] = duration + row_data["remaining"] = num_requests - len(relevant_history) + if relevant_history and len(relevant_history) >= num_requests: + # At this point, the endpoint becomes available again + now_request = relevant_history[-1] + remaining = duration - (now - int(now_request)) + row_data["available"] = int(now + remaining) or None + row_data["available_seconds"] = int(remaining) or None + else: + row_data["available"] = None + row_data["available_seconds"] = None + + if relevant_history: + # At this point, all Rate Limit is reset to 0 + latest_request = relevant_history[0] + remaining = duration - (now - int(latest_request)) + row_data["reset"] = int(now + remaining) + row_data["reset_seconds"] = int(remaining) + else: + row_data["reset"] = None + row_data["reset_seconds"] = None + else: + row_data["limit"] = None + row_data["duration"] = None + row_data["remaining"] = None + row_data["available"] = None + row_data["available_seconds"] = None + row_data["reset"] = None + row_data["reset_seconds"] = None + + data.append(row_data) + + return data + + +class FunkwhaleThrottle(rest_throttling.SimpleRateThrottle): + def __init__(self): + pass + + def get_cache_key(self, request, view): + return get_cache_key(self.scope, self.ident) + + def allow_request(self, request, view): + self.request = request + self.ident = get_ident(request) + action = getattr(view, "action", "*") + view_scopes = getattr(view, "throttling_scopes", {}) + if view_scopes is None: + return True + self.scope = get_scope_for_action_and_ident_type( + action=action, ident_type=self.ident["type"], view_conf=view_scopes + ) + if not self.scope or self.scope not in settings.THROTTLING_RATES: + return True + self.rate = settings.THROTTLING_RATES[self.scope].get("rate") + self.num_requests, self.duration = self.parse_rate(self.rate) + self.request = request + + return super().allow_request(request, view) + + def attach_info(self): + info = { + "num_requests": self.num_requests, + "duration": self.duration, + "scope": self.scope, + "history": self.history or [], + "wait": self.wait(), + } + setattr(self.request, "_throttle_status", info) + + def throttle_success(self): + self.attach_info() + return super().throttle_success() + + def throttle_failure(self): + self.attach_info() + return super().throttle_failure() + + +class TooManyRequests(Exception): + pass + + +DummyView = collections.namedtuple("DummyView", "action throttling_scopes") + + +def check_request(request, scope): + """ + A simple wrapper around FunkwhaleThrottle for views that aren't API views + or cannot use rest_framework automatic throttling. + + Raise TooManyRequests if limit is reached. + """ + if not settings.THROTTLING_ENABLED: + return True + + view = DummyView( + action=scope, + throttling_scopes={scope: {"anonymous": scope, "authenticated": scope}}, + ) + throttle = FunkwhaleThrottle() + if not throttle.allow_request(request, view): + raise TooManyRequests() + return True diff --git a/api/funkwhale_api/common/views.py b/api/funkwhale_api/common/views.py index db39c56d1..d197dbe90 100644 --- a/api/funkwhale_api/common/views.py +++ b/api/funkwhale_api/common/views.py @@ -1,3 +1,6 @@ +import time + +from django.conf import settings from django.db import transaction from rest_framework.decorators import action @@ -5,6 +8,7 @@ from rest_framework import exceptions from rest_framework import mixins from rest_framework import permissions from rest_framework import response +from rest_framework import views from rest_framework import viewsets from . import filters @@ -13,6 +17,7 @@ from . import mutations from . import serializers from . import signals from . import tasks +from . import throttling from . import utils @@ -121,3 +126,17 @@ class MutationViewSet( new_is_approved=instance.is_approved, ) return response.Response({}, status=200) + + +class RateLimitView(views.APIView): + permission_classes = [] + throttle_classes = [] + + def get(self, request, *args, **kwargs): + ident = throttling.get_ident(request) + data = { + "enabled": settings.THROTTLING_ENABLED, + "ident": ident, + "scopes": throttling.get_status(ident, time.time()), + } + return response.Response(data, status=200) diff --git a/api/funkwhale_api/moderation/views.py b/api/funkwhale_api/moderation/views.py index e37251078..67de68001 100644 --- a/api/funkwhale_api/moderation/views.py +++ b/api/funkwhale_api/moderation/views.py @@ -49,6 +49,12 @@ class ReportsViewSet(mixins.CreateModelMixin, viewsets.GenericViewSet): ordering_fields = ("creation_date",) anonymous_policy = "setting" anonymous_scopes = {"write:reports"} + throttling_scopes = { + "create": { + "anonymous": "anonymous-reports", + "authenticated": "authenticated-reports", + } + } def get_serializer_context(self): context = super().get_serializer_context() diff --git a/api/funkwhale_api/users/jwt_views.py b/api/funkwhale_api/users/jwt_views.py new file mode 100644 index 000000000..532653abd --- /dev/null +++ b/api/funkwhale_api/users/jwt_views.py @@ -0,0 +1,15 @@ +from rest_framework_jwt import views as jwt_views + + +class ObtainJSONWebToken(jwt_views.ObtainJSONWebToken): + throttling_scopes = {"*": {"anonymous": "jwt-login", "authenticated": "jwt-login"}} + + +class RefreshJSONWebToken(jwt_views.RefreshJSONWebToken): + throttling_scopes = { + "*": {"anonymous": "jwt-refresh", "authenticated": "jwt-refresh"} + } + + +obtain_jwt_token = ObtainJSONWebToken.as_view() +refresh_jwt_token = RefreshJSONWebToken.as_view() diff --git a/api/funkwhale_api/users/oauth/views.py b/api/funkwhale_api/users/oauth/views.py index a8bbb239c..90706e708 100644 --- a/api/funkwhale_api/users/oauth/views.py +++ b/api/funkwhale_api/users/oauth/views.py @@ -10,6 +10,8 @@ from oauth2_provider import exceptions as oauth2_exceptions from oauth2_provider import views as oauth_views from oauth2_provider.settings import oauth2_settings +from funkwhale_api.common import throttling + from .. import models from .permissions import ScopePermission from . import serializers @@ -35,6 +37,12 @@ class ApplicationViewSet( lookup_field = "client_id" queryset = models.Application.objects.all().order_by("-created") serializer_class = serializers.ApplicationSerializer + throttling_scopes = { + "create": { + "anonymous": "anonymous-oauth-app", + "authenticated": "authenticated-oauth-app", + } + } def get_serializer_class(self): if self.request.method.lower() == "post": @@ -141,6 +149,10 @@ class AuthorizeView(views.APIView, oauth_views.AuthorizationView): return self.json_payload(errors, status_code=400) + def post(self, request, *args, **kwargs): + throttling.check_request(request, "oauth-authorize") + return super().post(request, *args, **kwargs) + def form_valid(self, form): try: response = super().form_valid(form) @@ -175,8 +187,12 @@ class AuthorizeView(views.APIView, oauth_views.AuthorizationView): class TokenView(oauth_views.TokenView): - pass + def post(self, request, *args, **kwargs): + throttling.check_request(request, "oauth-token") + return super().post(request, *args, **kwargs) class RevokeTokenView(oauth_views.RevokeTokenView): - pass + def post(self, request, *args, **kwargs): + throttling.check_request(request, "oauth-revoke-token") + return super().post(request, *args, **kwargs) diff --git a/api/funkwhale_api/users/rest_auth_urls.py b/api/funkwhale_api/users/rest_auth_urls.py index 0421473ab..75ba608bb 100644 --- a/api/funkwhale_api/users/rest_auth_urls.py +++ b/api/funkwhale_api/users/rest_auth_urls.py @@ -1,20 +1,40 @@ from django.conf.urls import url from django.views.generic import TemplateView from rest_auth import views as rest_auth_views -from rest_auth.registration import views as registration_views from . import views urlpatterns = [ - url(r"^$", views.RegisterView.as_view(), name="rest_register"), + # URLs that do not require a session or valid token url( - r"^verify-email/?$", - registration_views.VerifyEmailView.as_view(), + r"^password/reset/$", + views.PasswordResetView.as_view(), + name="rest_password_reset", + ), + url( + r"^password/reset/confirm/$", + views.PasswordResetConfirmView.as_view(), + name="rest_password_reset_confirm", + ), + # URLs that require a user to be logged in with a valid session / token. + url( + r"^user/$", rest_auth_views.UserDetailsView.as_view(), name="rest_user_details" + ), + url( + r"^password/change/$", + views.PasswordChangeView.as_view(), + name="rest_password_change", + ), + # Registration URLs + url(r"^registration/$", views.RegisterView.as_view(), name="rest_register"), + url( + r"^registration/verify-email/?$", + views.VerifyEmailView.as_view(), name="rest_verify_email", ), url( - r"^change-password/?$", - rest_auth_views.PasswordChangeView.as_view(), + r"^registration/change-password/?$", + views.PasswordChangeView.as_view(), name="change_password", ), # This url is used by django-allauth and empty TemplateView is @@ -28,7 +48,7 @@ urlpatterns = [ # view from: # djang-allauth https://github.com/pennersr/django-allauth/blob/master/allauth/account/views.py#L190 url( - r"^account-confirm-email/(?P<key>\w+)/?$", + r"^registration/account-confirm-email/(?P<key>\w+)/?$", TemplateView.as_view(), name="account_confirm_email", ), diff --git a/api/funkwhale_api/users/views.py b/api/funkwhale_api/users/views.py index 8cbb23bd1..7cdb7cdbb 100644 --- a/api/funkwhale_api/users/views.py +++ b/api/funkwhale_api/users/views.py @@ -1,5 +1,6 @@ from allauth.account.adapter import get_adapter -from rest_auth.registration.views import RegisterView as BaseRegisterView +from rest_auth import views as rest_auth_views +from rest_auth.registration import views as registration_views from rest_framework import mixins, viewsets from rest_framework.decorators import action from rest_framework.response import Response @@ -9,9 +10,11 @@ from funkwhale_api.common import preferences from . import models, serializers -class RegisterView(BaseRegisterView): +class RegisterView(registration_views.RegisterView): serializer_class = serializers.RegisterSerializer permission_classes = [] + action = "signup" + throttling_scopes = {"signup": {"authenticated": "signup", "anonymous": "signup"}} def create(self, request, *args, **kwargs): invitation_code = request.data.get("invitation") @@ -24,6 +27,22 @@ class RegisterView(BaseRegisterView): return get_adapter().is_open_for_signup(request) +class VerifyEmailView(registration_views.VerifyEmailView): + action = "verify-email" + + +class PasswordChangeView(rest_auth_views.PasswordChangeView): + action = "password-change" + + +class PasswordResetView(rest_auth_views.PasswordResetView): + action = "password-reset" + + +class PasswordResetConfirmView(rest_auth_views.PasswordResetConfirmView): + action = "password-reset-confirm" + + class UserViewSet(mixins.UpdateModelMixin, viewsets.GenericViewSet): queryset = models.User.objects.all() serializer_class = serializers.UserWriteSerializer diff --git a/api/tests/common/test_middleware.py b/api/tests/common/test_middleware.py index 9361748f3..c62add988 100644 --- a/api/tests/common/test_middleware.py +++ b/api/tests/common/test_middleware.py @@ -1,6 +1,10 @@ +import time import pytest +from django.http import HttpResponse + from funkwhale_api.common import middleware +from funkwhale_api.common import throttling def test_spa_fallback_middleware_no_404(mocker): @@ -185,3 +189,39 @@ def test_get_custom_css(preferences, custom_css, expected): preferences["ui__custom_css"] = custom_css assert middleware.get_custom_css() == expected + + +def test_throttle_status_middleware_includes_info_in_response_headers(mocker): + get_response = mocker.Mock() + response = HttpResponse() + get_response.return_value = response + request = mocker.Mock( + path="/", + _api_request=mocker.Mock( + _throttle_status={ + "num_requests": 42, + "duration": 3600, + "scope": "hello", + "history": [time.time() - 1600, time.time() - 1800], + } + ), + ) + m = middleware.ThrottleStatusMiddleware(get_response) + + assert m(request) == response + assert response["X-RateLimit-Limit"] == "42" + assert response["X-RateLimit-Remaining"] == "40" + assert response["X-RateLimit-Duration"] == "3600" + assert response["X-RateLimit-Scope"] == "hello" + assert response["X-RateLimit-Reset"] == str(int(time.time()) + 2000) + assert response["X-RateLimit-ResetSeconds"] == str(2000) + assert response["Retry-After"] == str(1800) + + +def test_throttle_status_middleware_returns_proper_response(mocker): + get_response = mocker.Mock(side_effect=throttling.TooManyRequests()) + request = mocker.Mock(path="/", _api_request=None, _throttle_status=None) + m = middleware.ThrottleStatusMiddleware(get_response) + + response = m(request) + assert response.status_code == 429 diff --git a/api/tests/common/test_throttling.py b/api/tests/common/test_throttling.py new file mode 100644 index 000000000..6b000fcaf --- /dev/null +++ b/api/tests/common/test_throttling.py @@ -0,0 +1,337 @@ +import time +import pytest + +from funkwhale_api.common import throttling + + +def test_get_ident_anonymous(api_request): + ip = "92.92.92.92" + request = api_request.get("/", HTTP_X_FORWARDED_FOR=ip) + + expected = {"id": ip, "type": "anonymous"} + + assert throttling.get_ident(request) == expected + + +def test_get_ident_authenticated(api_request, factories): + user = factories["users.User"]() + request = api_request.get("/") + setattr(request, "user", user) + expected = {"id": user.pk, "type": "authenticated"} + assert throttling.get_ident(request) == expected + + +@pytest.mark.parametrize( + "scope, ident, expected", + [ + ( + "create", + {"id": 42, "type": "authenticated"}, + "throttling:create:authenticated:42", + ), + ( + "list", + {"id": "92.92.92.92", "type": "anonymous"}, + "throttling:list:anonymous:92.92.92.92", + ), + ], +) +def test_get_cache_key(scope, ident, expected): + assert throttling.get_cache_key(scope, ident) == expected + + +@pytest.mark.parametrize( + "action, type, view_conf, throttling_actions, expected", + [ + # exact match, we return the rate + ("retrieve", "anonymous", {}, {"retrieve": {"anonymous": "test"}}, "test"), + # exact match on the view, we return the rate + ("retrieve", "anonymous", {"retrieve": {"anonymous": "test"}}, {}, "test"), + # no match, we return nothing + ("retrieve", "authenticated", {}, {}, None), + ("retrieve", "authenticated", {}, {"retrieve": {"anonymous": "test"}}, None), + ( + "retrieve", + "authenticated", + {"destroy": {"authenticated": "test"}}, + {"retrieve": {"anonymous": "test"}}, + None, + ), + # exact match on the view, and in the settings, the view is more important + ( + "retrieve", + "anonymous", + {"retrieve": {"anonymous": "test"}}, + {"retrieve": {"anonymous": "test-2"}}, + "test", + ), + # wildcard match, we return the wildcard value + ("retrieve", "authenticated", {}, {"*": {"authenticated": "test"}}, "test"), + # wildcard match, but more specific match also, we use this one instead + ( + "retrieve", + "authenticated", + {}, + {"retrieve": {"authenticated": "test-2"}, "*": {"authenticated": "test"}}, + "test-2", + ), + ], +) +def test_get_rate_for_scope_and_ident_type( + action, type, view_conf, throttling_actions, expected, settings +): + settings.THROTTLING_SCOPES = throttling_actions + assert ( + throttling.get_scope_for_action_and_ident_type( + action=action, ident_type=type, view_conf=view_conf + ) + is expected + ) + + +@pytest.mark.parametrize( + "view_args, throttling_rates, previous_requests, expected", + [ + # room for one more requests + ( + { + "action": "retrieve", + "throttling_scopes": {"retrieve": {"anonymous": "test"}}, + }, + {"test": {"rate": "3/s"}}, + 2, + True, + ), + # number of requests exceeded + ( + { + "action": "retrieve", + "throttling_scopes": {"retrieve": {"anonymous": "test"}}, + }, + {"test": {"rate": "3/s"}}, + 3, + False, + ), + # no throttling setup + ( + { + "action": "delete", + "throttling_scopes": {"retrieve": {"anonymous": "test"}}, + }, + {}, + 1000, + True, + ), + ], +) +def test_throttle_anonymous( + view_args, + throttling_rates, + previous_requests, + expected, + api_request, + mocker, + settings, +): + settings.THROTTLING_RATES = throttling_rates + settings.THROTTLING_SCOPES = {} + ip = "92.92.92.92" + ident = {"type": "anonymous", "id": ip} + request = api_request.get("/", HTTP_X_FORWARDED_FOR=ip) + + view = mocker.Mock(**view_args) + + cache_key = throttling.get_cache_key("test", ident) + throttle = throttling.FunkwhaleThrottle() + + history = [time.time() for _ in range(previous_requests)] + throttle.cache.set(cache_key, history) + + assert throttle.allow_request(request, view) is expected + + +@pytest.mark.parametrize( + "view_args, throttling_rates, previous_requests, expected", + [ + # room for one more requests + ( + { + "action": "retrieve", + "throttling_scopes": {"retrieve": {"authenticated": "test"}}, + }, + {"test": {"rate": "3/s"}}, + 2, + True, + ), + # number of requests exceeded + ( + { + "action": "retrieve", + "throttling_scopes": {"retrieve": {"authenticated": "test"}}, + }, + {"test": {"rate": "3/s"}}, + 3, + False, + ), + # no throttling setup + ( + { + "action": "delete", + "throttling_scopes": {"retrieve": {"authenticated": "test"}}, + }, + {}, + 1000, + True, + ), + ], +) +def test_throttle_authenticated( + view_args, + throttling_rates, + previous_requests, + expected, + api_request, + mocker, + settings, + factories, +): + settings.THROTTLING_RATES = throttling_rates + settings.THROTTLING_SCOPES = {} + user = factories["users.User"]() + ident = {"type": "authenticated", "id": user.pk} + request = api_request.get("/") + setattr(request, "user", user) + + view = mocker.Mock(**view_args) + + cache_key = throttling.get_cache_key("test", ident) + throttle = throttling.FunkwhaleThrottle() + + history = [time.time() for _ in range(previous_requests)] + throttle.cache.set(cache_key, history) + + assert throttle.allow_request(request, view) is expected + + +def throttle_successive(settings, mocker, api_request): + settings.THROTTLING_RATES = {"test": {"rate": "3/s"}} + settings.THROTTLING_SCOPES = {} + ip = "92.92.92.92" + request = api_request.get("/", HTTP_X_FORWARDED_FOR=ip) + + view = mocker.Mock( + action="retrieve", throttling_scopes={"retrieve": {"anonymous": "test"}} + ) + + throttle = throttling.FunkwhaleThrottle() + + assert throttle.allow_request(request, view) is True + assert throttle.allow_request(request, view) is True + assert throttle.allow_request(request, view) is True + assert throttle.allow_request(request, view) is False + + +def test_throttle_attach_info(mocker): + throttle = throttling.FunkwhaleThrottle() + request = mocker.Mock() + setattr(throttle, "num_requests", 300) + setattr(throttle, "duration", 3600) + setattr(throttle, "scope", "hello") + setattr(throttle, "history", []) + setattr(throttle, "request", request) + + expected = { + "num_requests": throttle.num_requests, + "duration": throttle.duration, + "history": throttle.history, + "wait": throttle.wait(), + "scope": throttle.scope, + } + throttle.attach_info() + + assert request._throttle_status == expected + + +@pytest.mark.parametrize("method", ["throttle_success", "throttle_failure"]) +def test_throttle_calls_attach_info(method, mocker): + throttle = throttling.FunkwhaleThrottle() + setattr(throttle, "key", "noop") + setattr(throttle, "now", "noop") + setattr(throttle, "duration", "noop") + setattr(throttle, "history", ["noop"]) + mocker.patch.object(throttle, "cache") + attach_info = mocker.patch.object(throttle, "attach_info") + func = getattr(throttle, method) + + func() + + attach_info.assert_called_once_with() + + +def test_allow_request(api_request, settings, mocker): + settings.THROTTLING_RATES = {"test": {"rate": "2/s"}} + ip = "92.92.92.92" + request = api_request.get("/", HTTP_X_FORWARDED_FOR=ip) + allow_request = mocker.spy(throttling.FunkwhaleThrottle, "allow_request") + action = "test" + throttling_scopes = {"test": {"anonymous": "test", "authenticated": "test"}} + throttling.check_request(request, action) + throttling.check_request(request, action) + with pytest.raises(throttling.TooManyRequests): + throttling.check_request(request, action) + + assert allow_request.call_count == 3 + assert allow_request.call_args[0][1] == request + assert allow_request.call_args[0][2] == throttling.DummyView( + action=action, throttling_scopes=throttling_scopes + ) + + +def test_allow_request_throttling_disabled(api_request, settings): + settings.THROTTLING_RATES = {"test": {"rate": "1/s"}} + settings.THROTTLING_ENABLED = False + ip = "92.92.92.92" + request = api_request.get("/", HTTP_X_FORWARDED_FOR=ip) + action = "test" + throttling.check_request(request, action) + # even exceeding request doesn't raise any exception + throttling.check_request(request, action) + + +def test_get_throttling_status_for_ident(settings, cache): + settings.THROTTLING_RATES = { + "test-1": {"rate": "30/d", "description": "description 1"}, + "test-2": {"rate": "20/h", "description": "description 2"}, + } + ident = {"type": "anonymous", "id": "92.92.92.92"} + test1_cache_key = throttling.get_cache_key("test-1", ident) + now = int(time.time()) + cache.set(test1_cache_key, [now - 1, now - 2, now - 99999999]) + + expected = [ + { + "id": "test-1", + "limit": 30, + "rate": "30/d", + "description": "description 1", + "duration": 24 * 3600, + "remaining": 28, + "reset": now + (24 * 3600) - 1, + "reset_seconds": (24 * 3600) - 1, + "available": None, + "available_seconds": None, + }, + { + "id": "test-2", + "limit": 20, + "rate": "20/h", + "description": "description 2", + "duration": 3600, + "remaining": 20, + "reset": None, + "reset_seconds": None, + "available": None, + "available_seconds": None, + }, + ] + assert throttling.get_status(ident, now) == expected diff --git a/api/tests/common/test_views.py b/api/tests/common/test_views.py index d2b53b41f..c1cbfd761 100644 --- a/api/tests/common/test_views.py +++ b/api/tests/common/test_views.py @@ -4,6 +4,7 @@ from django.urls import reverse from funkwhale_api.common import serializers from funkwhale_api.common import signals from funkwhale_api.common import tasks +from funkwhale_api.common import throttling def test_can_detail_mutation(logged_in_api_client, factories): @@ -163,3 +164,20 @@ def test_cannot_approve_reject_without_perm( assert mutation.is_approved is None assert mutation.approved_by is None + + +def test_rate_limit(logged_in_api_client, now_time, settings, mocker): + expected_ident = {"type": "authenticated", "id": logged_in_api_client.user.pk} + + expected = { + "ident": expected_ident, + "scopes": throttling.get_status(expected_ident, now_time), + "enabled": settings.THROTTLING_ENABLED, + } + get_status = mocker.spy(throttling, "get_status") + url = reverse("api:v1:rate-limit") + response = logged_in_api_client.get(url) + + assert response.status_code == 200 + assert response.data == expected + get_status.assert_called_once_with(expected_ident, now_time) diff --git a/api/tests/conftest.py b/api/tests/conftest.py index bf9922377..a7fa02cc0 100644 --- a/api/tests/conftest.py +++ b/api/tests/conftest.py @@ -5,6 +5,7 @@ import PIL import random import shutil import tempfile +import time import factory import pytest @@ -308,6 +309,13 @@ def now(mocker): return now +@pytest.fixture() +def now_time(mocker): + now = time.time() + mocker.patch("time.time", return_value=now) + return now + + @pytest.fixture() def avatar(): i = PIL.Image.new("RGBA", (400, 400), random.choice(["red", "blue", "yellow"])) diff --git a/changes/changelog.d/261.feature b/changes/changelog.d/261.feature new file mode 100644 index 000000000..8d5e7139b --- /dev/null +++ b/changes/changelog.d/261.feature @@ -0,0 +1 @@ +Enforce a configurable rate limit on the API to mitigate abuse (#261) diff --git a/changes/notes.rst b/changes/notes.rst index 41af5ed9c..d7324a31b 100644 --- a/changes/notes.rst +++ b/changes/notes.rst @@ -149,3 +149,15 @@ Then reload nginx with ``systemctl reload nginx``. } Then reload nginx with ``docker-compose restart nginx``. + +Rate limiting +^^^^^^^^^^^^^ + +With this release, rate-limiting on the API is enabled by default, with high enough limits to ensure +regular users of the app aren't affected. Requests beyond allowed limits are answered with a 429 HTTP error. + +For anonymous requests, the limit is applied to the IP adress of the client, and for authenticated requests, the limit +is applied to the corresponding user account. By default, anonymous requests get a lower limit than authenticated requests. + +You can disable the rate-limiting feature by adding `THROTTLING_ENABLED=false` to your ``.env`` file and restarting the +services. If you are using the Funkwhale API in your project or app and want to know more about the limits, please consult https://docs.funkwhale.audio/swagger/. diff --git a/docs/swagger.yml b/docs/swagger.yml index d7a0230c0..a6952098b 100644 --- a/docs/swagger.yml +++ b/docs/swagger.yml @@ -33,6 +33,79 @@ info: If you keep the default server (https://demo.funkwhale.audio), the default username and password couple is "demo" and "demo". + Rate limiting + ------------- + + Depending on server configuration, pods running Funkwhale 0.20 and higher may rate-limit incoming + requests to prevent abuse and improve the stability of service. Requests that are dropped because of rate-limiting + receive a 429 HTTP response. + + The limits themselves vary depending on: + + - The client: anonymous requests are subject to lower limits than authenticated requests + - The operation being performed: Write and delete operations, as performed with DELETE, POST, PUT and PATCH HTTP methods are subject to lower limits + + Those conditions are used to determine the scope of the request, which in turns determine the limit that is applied. + For instance, authenticated POST requests are bound to the `authenticated-create` scope, with a default limit of + 1000 requests/hour, but anonymous POST requests are bound to the `anonymous-create` scope, with a lower limit of 1000 requests/day. + + A full list of scopes with their corresponding description, and the current usage data for the client performing the request + is available via the `/api/v1/rate-limit` endpoint. + + Additionally, we include HTTP headers on all API response to ensure API clients can understand: + + - what scope was bound to a given request + - what is the corresponding limit + - how much similar requests can be sent before being limited + - and how much time they should wait if they have been limited + + <table> + <caption>Rate limiting headers</caption> + <thead> + <th>Header</th> + <th>Example value</th> + <th>Description value</th> + </thead> + <tbody> + <tr> + <td><code>X-RateLimit-Limit</code></td> + <td>50</td> + <td>The number of allowed requests whithin a given period</td> + </tr> + <tr> + <td><code>X-RateLimit-Duration</code></td> + <td>3600</td> + <td>The time window, in seconds, during which those requests are accounted for.</td> + </tr> + <tr> + <td><code>X-RateLimit-Scope</code></td> + <td>login</td> + <td>The name of the scope as computed for the request</td> + </tr> + <tr> + <td><code>X-RateLimit-Remaining</code></td> + <td>42</td> + <td>How many requests can be sent with the same scope before the limit applies</td> + </tr> + <tr> + <td><code>Retry-After</code> (if <code>X-RateLimit-Remaining</code> is 0)</td> + <td>3543</td> + <td>How many seconds to wait before a retry</td> + </tr> + <tr> + <td><code>X-RateLimit-Reset</code></td> + <td>1568126089</td> + <td>A timestamp indicating when <code>X-RateLimit-Remaining</code> will return to its higher possible value</td> + </tr> + <tr> + <td><code>X-RateLimit-ResetSeconds</code></td> + <td>3599</td> + <td>How many seconds to wait before <code>X-RateLimit-Remaining</code> returns to its higher possible value</td> + </tr> + </tbody> + </table> + + Resources --------- @@ -103,7 +176,7 @@ security: tags: - name: Auth and security - description: Login, logout and authorization endpoints + description: Login, logout, rate-limit and authorization endpoints - name: Library and metadata description: Information and metadata about musical and audio entities (albums, tracks, artists, etc.) - name: Uploads and audio content @@ -117,7 +190,7 @@ paths: /api/v1/oauth/apps/: post: tags: - - "auth" + - "Auth and security" description: Register an OAuth application security: [] @@ -247,6 +320,19 @@ paths: schema: $ref: "#/definitions/Me" + /api/v1/rate-limit/: + get: + summary: Retrive rate-limit information and current usage status + tags: + - "Auth and security" + + responses: + 200: + content: + application/json: + schema: + $ref: "#/definitions/RateLimitStatus" + /api/v1/artists/: get: summary: List artists @@ -1646,6 +1732,75 @@ definitions: type: "boolean" example: false description: A boolean indicating if the user can manage instance settings and users + RateLimitStatus: + type: "object" + properties: + enabled: + type: "boolean" + example: true + description: A boolean indicating if rate-limiting is enabled on the server + ident: + type: "object" + description: Client-related data + properties: + type: + type: string + example: "anonymous" + enum: + - "authenticated" + - "anonymous" + id: + type: string + example: "92.143.42" + description: An address IP or user ID identifying the client + scopes: + type: "array" + items: + type: "object" + description: Rate-limit scope configuration and usage + properties: + id: + type: string + example: "password-reset" + description: + type: string + example: "Password reset request" + rate: + type: string + example: "30/day" + limit: + type: "integer" + format: "int64" + example: 30 + duration: + type: "integer" + format: "int64" + example: 86400 + remaining: + type: "integer" + format: "int64" + example: 28 + description: How many requests can be sent with the same scope before the limit applies + reset: + type: "integer" + format: "int64" + example: 1568126189 + description: A timestamp indicating when <code>remaining</code> will return to its higher possible value + reset_seconds: + type: "integer" + format: "int64" + example: 86267 + description: How many seconds to wait before <code>remaining</code> returns to its higher possible value + available: + type: "integer" + format: "int64" + example: 1568126089 + description: A timestamp indicating when the client can retry + available_seconds: + type: "integer" + format: "int64" + example: 54 + description: How many seconds to wait before a retry ResourceNotFound: type: "object" diff --git a/front/src/components/auth/Login.vue b/front/src/components/auth/Login.vue index 11d4551da..0fb391654 100644 --- a/front/src/components/auth/Login.vue +++ b/front/src/components/auth/Login.vue @@ -8,7 +8,7 @@ <div class="header"><translate translate-context="Content/Login/Error message.Title">We cannot log you in</translate></div> <ul class="list"> <li v-if="error == 'invalid_credentials'"><translate translate-context="Content/Login/Error message.List item/Call to action">Please double-check your username/password couple is correct</translate></li> - <li v-else><translate translate-context="Content/Login/Error message/List item">An unknown error occurred, this can mean the server is down or cannot be reached</translate></li> + <li v-else>{{ error }}</li> </ul> </div> <div class="field"> @@ -105,7 +105,7 @@ export default { if (error.response.status === 400) { self.error = "invalid_credentials" } else { - self.error = "unknown_error" + self.error = error.backendErrors[0] } } }) diff --git a/front/src/main.js b/front/src/main.js index 145fa6111..f58cdbdbc 100644 --- a/front/src/main.js +++ b/front/src/main.js @@ -7,6 +7,7 @@ logger.default.debug('Environment variables:', process.env) import jQuery from "jquery" import Vue from 'vue' +import moment from 'moment' import App from './App' import router from './router' import axios from 'axios' @@ -25,6 +26,7 @@ sync(store, router) window.$ = window.jQuery = require('jquery') require('./semantic.js') require('masonry-layout') +let APP = null let availableLanguages = (function () { let l = {} @@ -91,6 +93,32 @@ axios.interceptors.response.use(function (response) { error.backendErrors.push('Resource not found') } else if (error.response.status === 403) { error.backendErrors.push('Permission denied') + } else if (error.response.status === 429) { + let message + let rateLimitStatus = { + limit: error.response.headers['x-ratelimit-limit'], + scope: error.response.headers['x-ratelimit-scope'], + remaining: error.response.headers['x-ratelimit-remaining'], + duration: error.response.headers['x-ratelimit-duration'], + availableSeconds: error.response.headers['retry-after'], + reset: error.response.headers['x-ratelimit-reset'], + resetSeconds: error.response.headers['x-ratelimit-resetseconds'], + } + if (rateLimitStatus.availableSeconds) { + rateLimitStatus.availableSeconds = parseInt(rateLimitStatus.availableSeconds) + let tryAgain = moment().add(rateLimitStatus.availableSeconds, 's').toNow(true) + message = APP.$pgettext('*/Error/Paragraph', 'You sent too many requests and have been rate limited, please try again in %{ delay }') + message = APP.$gettextInterpolate(message, {delay: tryAgain}) + } else { + message = APP.$pgettext('*/Error/Paragraph', 'You sent too many requests and have been rate limited, please try again later') + } + error.backendErrors.push(message) + store.commit("ui/addMessage", { + content: message, + date: new Date(), + level: 'error', + }) + logger.default.error('This client is rate-limited!', rateLimitStatus) } else if (error.response.status === 500) { error.backendErrors.push('A server error occured') } else if (error.response.data) { @@ -125,7 +153,10 @@ store.dispatch('instance/fetchFrontSettings').finally(() => { render (h) { return h('App') }, - components: { App } + components: { App }, + created () { + APP = this + }, }) logger.default.info('Everything loaded!') -- GitLab