From ad8bdc14e6e0c4a80208fcbc4fdf5ba802e13c06 Mon Sep 17 00:00:00 2001 From: madewithkode Date: Wed, 15 May 2024 10:50:36 +0100 Subject: [PATCH] add 'revoked' throttle scope to enable easily revoking openverse TOS violating client application access. --- ...er_throttledapplication_rate_limit_model.py | 18 ++++++++++++++++++ api/api/models/oauth.py | 1 + api/api/utils/throttle.py | 5 +++++ api/conf/oauth2_extensions.py | 14 ++++++++++++-- api/conf/settings/rest_framework.py | 3 +++ api/latest_migrations/api | 2 +- api/test/integration/test_auth.py | 5 +++-- 7 files changed, 43 insertions(+), 5 deletions(-) create mode 100644 api/api/migrations/0062_alter_throttledapplication_rate_limit_model.py diff --git a/api/api/migrations/0062_alter_throttledapplication_rate_limit_model.py b/api/api/migrations/0062_alter_throttledapplication_rate_limit_model.py new file mode 100644 index 00000000000..ea32cbfa07f --- /dev/null +++ b/api/api/migrations/0062_alter_throttledapplication_rate_limit_model.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.11 on 2024-05-15 09:43 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('api', '0061_convert_varchar_to_text'), + ] + + operations = [ + migrations.AlterField( + model_name='throttledapplication', + name='rate_limit_model', + field=models.CharField(choices=[('standard', 'standard'), ('enhanced', 'enhanced'), ('exempt', 'exempt'), ('revoked', 'revoked')], default='standard', max_length=20), + ), + ] diff --git a/api/api/models/oauth.py b/api/api/models/oauth.py index 50e6fb9201a..08e6912f0c2 100644 --- a/api/api/models/oauth.py +++ b/api/api/models/oauth.py @@ -33,6 +33,7 @@ class ThrottledApplication(AbstractApplication): # case-by-case basis. ("exempt", "exempt"), # Rate limits used for internal infrastructure to # by-pass rate limiting entirely. + ("revoked", "revoked"), # Rate limits used for outrightly disallowing access. ] rate_limit_model = models.CharField( max_length=20, choices=RATE_LIMIT_MODELS, default="standard" diff --git a/api/api/utils/throttle.py b/api/api/utils/throttle.py index e4de75f2c9c..5d272d5eda4 100644 --- a/api/api/utils/throttle.py +++ b/api/api/utils/throttle.py @@ -186,3 +186,8 @@ class EnhancedOAuth2IdBurstRateThrottle(AbstractOAuth2IdRateThrottle): class ExemptOAuth2IdRateThrottle(AbstractOAuth2IdRateThrottle): applies_to_rate_limit_model = {"exempt"} scope = "exempt_oauth2_client_credentials" + + +class RevokedOAuth2IdRateThrottle(AbstractOAuth2IdRateThrottle): + applies_to_rate_limit_model = {"revoked"} + scope = "revoked" diff --git a/api/conf/oauth2_extensions.py b/api/conf/oauth2_extensions.py index 2ece1b49777..6db95e53146 100644 --- a/api/conf/oauth2_extensions.py +++ b/api/conf/oauth2_extensions.py @@ -1,4 +1,7 @@ -from rest_framework.exceptions import AuthenticationFailed +from rest_framework.exceptions import ( + AuthenticationFailed, + PermissionDenied, +) from drf_spectacular.authentication import TokenScheme from oauth2_provider.contrib.rest_framework import ( @@ -18,7 +21,14 @@ def authenticate(self, request): # requests with valid credentials # `request` is mutated by `super().authenticate` raise AuthenticationFailed() - + # if this is an authed request, check and + # deny access if client's access has been + # revoked. + if getattr(request, "auth", None): + auth_user = getattr(request, "auth") + application = getattr(auth_user, "application", None) + if application and application.rate_limit_model == "revoked": + raise PermissionDenied() return result diff --git a/api/conf/settings/rest_framework.py b/api/conf/settings/rest_framework.py index 28252de1b27..5bcd19ffa53 100644 --- a/api/conf/settings/rest_framework.py +++ b/api/conf/settings/rest_framework.py @@ -38,6 +38,8 @@ "enhanced_oauth2_client_credentials_burst": "200/min", # ``None`` completely by-passes the rate limiting "exempt_oauth2_client_credentials": None, + # Outrightly deny access + "revoked": "0/day", } DEFAULT_THROTTLE_CLASSES = ( @@ -50,6 +52,7 @@ "api.utils.throttle.EnhancedOAuth2IdBurstRateThrottle", "api.utils.throttle.EnhancedOAuth2IdSustainedRateThrottle", "api.utils.throttle.ExemptOAuth2IdRateThrottle", + "api.utils.throttle.RevokedOAuth2IdRateThrottle", ) REST_FRAMEWORK = { diff --git a/api/latest_migrations/api b/api/latest_migrations/api index 56319b5792c..10cecf60932 100644 --- a/api/latest_migrations/api +++ b/api/latest_migrations/api @@ -2,4 +2,4 @@ # If you have a merge conflict in this file, it means you need to run: # manage.py makemigrations --merge # in order to resolve the conflict between migrations. -0061_convert_varchar_to_text +0062_alter_throttledapplication_rate_limit_model diff --git a/api/test/integration/test_auth.py b/api/test/integration/test_auth.py index f462368aca8..7ff9d76c655 100644 --- a/api/test/integration/test_auth.py +++ b/api/test/integration/test_auth.py @@ -94,7 +94,7 @@ def _integration_verify_most_recent_token(api_client): @pytest.mark.django_db @pytest.mark.parametrize( "rate_limit_model", - [x[0] for x in ThrottledApplication.RATE_LIMIT_MODELS], + [x[0] for x in ThrottledApplication.RATE_LIMIT_MODELS if not x[0] == "revoked"], ) @cache_availability_params @pytest.mark.skipif( @@ -129,7 +129,7 @@ def test_auth_email_verification( @pytest.mark.django_db @pytest.mark.parametrize( "rate_limit_model", - [x[0] for x in ThrottledApplication.RATE_LIMIT_MODELS], + [x[0] for x in ThrottledApplication.RATE_LIMIT_MODELS if not x[0] == "revoked"], ) @cache_availability_params def test_auth_rate_limit_reporting( @@ -267,6 +267,7 @@ def test_page_size_limit_authed(api_client, test_auth_token_exchange): res = api_client.get( "/v1/images/", query_params, HTTP_AUTHORIZATION=f"Bearer {token}" ) + assert res.status_code == 200 query_params = {"page_size": 500}