feat(api): Throttle dyndns API based on all domains given

Author: molikuner

api/desecapi/tests/test_throttling_scoped.py

@@ -42,6 +42,7 @@ class ThrottlingTestCase(TestCase):
     def setUp(self):
         super().setUp()
         self.factory = APIRequestFactory()
+        cache.clear()
 
     def _test_requests_are_throttled(self, rates, counts, buckets=None):
         def do_test():
@@ -65,16 +66,16 @@ def do_test():
                         max_wait - 1 <= float(response["Retry-After"]) <= max_wait
                     )
 
-        cache.clear()
         request = self.factory.get("/")
         with override_settings(
             REST_FRAMEWORK={"DEFAULT_THROTTLE_RATES": {MockView.throttle_scope: rates}}
         ):
-            do_test()
             if buckets is not None:
                 for bucket in buckets:
                     with override_bucket(bucket):
                         do_test()
+            else:
+                do_test()
 
     def test_requests_are_throttled_4sec(self):
         self._test_requests_are_throttled(["4/sec"], [(0, 4, 1), (1, 4, 1)])
@@ -85,8 +86,10 @@ def test_requests_are_throttled_4min(self):
     def test_requests_are_throttled_2per2min(self):
         self._test_requests_are_throttled(["2/2min"], [(0, 2, 120)])
 
-    def test_requests_are_throttled_multiple(self):
+    def test_requests_are_throttled_multiple_5sec_4day(self):
         self._test_requests_are_throttled(["5/s", "4/day"], [(0, 4, 86400)])
+
+    def test_requests_are_throttled_multiple_4sec_5day(self):
         self._test_requests_are_throttled(["4/s", "5/day"], [(0, 4, 1)])
 
     def test_requests_are_throttled_multiple_cascade(self):
@@ -98,3 +101,18 @@ def test_requests_are_throttled_multiple_cascade_with_buckets(self):
         self._test_requests_are_throttled(
             ["4/s", "6/day"], [(0, 4, 1), (1, 2, 86400)], buckets=["foo", "bar"]
         )
+
+    def test_requests_are_throttled_multiple_cascase_with_parallel_buckets(self):
+        # foo is thottled after 4 requests
+        self._test_requests_are_throttled(
+            ["4/s", "6/day"], [(0, 4, 1)], buckets=["foo"]
+        )
+        # a request using foo and bar is throttled (as foo is already throttled)
+        # but after foo is unthottled, 2 requests are allowed in the second second until foo is throttled again
+        self._test_requests_are_throttled(
+            ["4/s", "6/day"], [(0, 0, 1), (1, 2, 86400)], buckets=[{"foo", "bar"}]
+        )
+        # bar still can take 2 more requests in the second second and 2 more in the third second
+        self._test_requests_are_throttled(
+            ["4/s", "6/day"], [(1, 2, 1), (1, 2, 86400)], buckets=["bar"]
+        )

api/desecapi/throttling.py

@@ -39,45 +39,54 @@ def allow_request(self, request, view):
         if self.rate is None:
             return True
 
-        # Amend scope with optional bucket
-        bucket = getattr(view, self.scope_attr + "_bucket", None)
-        if bucket is not None:
-            self.scope += ":" + sha1(bucket.encode()).hexdigest()
+        buckets = getattr(view, self.scope_attr + "_bucket", None)
+        if not isinstance(buckets, set):
+            buckets = {buckets}
 
         self.now = self.timer()
         self.num_requests, self.duration = zip(*self.parse_rate(self.rate))
-        self.key = self.get_cache_key(request, view)
-        self.history = {key: [] for key in self.key}
-        self.history.update(self.cache.get_many(self.key))
-
-        for num_requests, duration, key in zip(
-            self.num_requests, self.duration, self.key
-        ):
-            history = self.history[key]
-            # Drop any requests from the history which have now passed the
-            # throttle duration
-            while history and history[-1] <= self.now - duration:
-                history.pop()
-            if len(history) >= num_requests:
-                # Prepare variables used by the Throttle's wait() method that gets called by APIView.check_throttles()
-                self.num_requests, self.duration, self.key, self.history = (
-                    num_requests,
-                    duration,
-                    key,
-                    history,
-                )
-                response = self.throttle_failure()
-                metrics.get("desecapi_throttle_failure").labels(
-                    request.method, scope, request.user.pk, bucket
-                ).inc()
-                return response
-            self.history[key] = history
+        self.histories = {}
+        for bucket in buckets:
+            # Amend scope with optional bucket
+            if bucket is not None:
+                self.scope = scope + ":" + sha1(bucket.encode()).hexdigest()
+            else:
+                self.scope = scope
+
+            self.key = self.get_cache_key(request, view)
+            bucket_history = {key: [] for key in self.key}
+            bucket_history.update(self.cache.get_many(self.key))
+
+            for num_requests, duration, key in zip(
+                self.num_requests, self.duration, self.key
+            ):
+                history = bucket_history[key]
+                # Drop any requests from the history which have now passed the
+                # throttle duration
+                while history and history[-1] <= self.now - duration:
+                    history.pop()
+                if len(history) >= num_requests:
+                    # Prepare variables used by the Throttle's wait() method that gets called by APIView.check_throttles()
+                    self.num_requests, self.duration, self.key, self.history = (
+                        num_requests,
+                        duration,
+                        key,
+                        history,
+                    )
+                    response = self.throttle_failure()
+                    metrics.get("desecapi_throttle_failure").labels(
+                        request.method, scope, request.user.pk, bucket
+                    ).inc()
+                    return response
+                bucket_history[key] = history
+            self.histories[bucket] = bucket_history
         return self.throttle_success()
 
     def throttle_success(self):
-        for key in self.history:
-            self.history[key].insert(0, self.now)
-        self.cache.set_many(self.history, max(self.duration))
+        for bucket_history in self.histories.values():
+            for history in bucket_history.values():
+                history.insert(0, self.now)
+            self.cache.set_many(bucket_history, max(self.duration))
         return True
 
     # Override the static attribute of the parent class so that we can dynamically apply override settings for testing

api/desecapi/views/dyndns.py

@@ -73,7 +73,7 @@ class DynDNS12UpdateView(generics.GenericAPIView):
 
     @property
     def throttle_scope_bucket(self):
-        return ",".join(sorted([d.name for d in self.domains]))
+        return {d.name for d in self.domains}
 
     def _find_action(
         self, param_keys, separator, use_remote_ip_fallback=False