Also bypass group rate limits

Author: timokoessler

aikido_zen/ratelimiting/__init__.py

@@ -24,6 +24,12 @@ def should_ratelimit_request(
     max_requests = int(endpoint["rateLimiting"]["maxRequests"])
     windows_size_in_ms = int(endpoint["rateLimiting"]["windowSizeInMS"])
 
+    if (
+        user
+        and user.get("id") in connection_manager.conf.excluded_uids_from_rate_limiting
+    ):
+        return {"block": False}
+
     if group:
         allowed = connection_manager.rate_limiter.is_allowed(
             get_key_for_group(endpoint, group),
@@ -36,8 +42,6 @@ def should_ratelimit_request(
         # Do not check IP or user rate limit if group is set
         return {"block": False}
     if user:
-        if user.get("id") in connection_manager.conf.excluded_uids_from_rate_limiting:
-            return {"block": False}
         allowed = connection_manager.rate_limiter.is_allowed(
             get_key_for_user(endpoint, user),
             windows_size_in_ms,

aikido_zen/ratelimiting/init_test.py

@@ -531,7 +531,7 @@ def test_non_excluded_user_still_rate_limited():
     }
 
 
-def test_excluded_user_still_blocked_by_group_rate_limit():
+def test_excluded_user_bypasses_group_rate_limit():
     endpoint = {
         "method": "POST",
         "route": "/login",
@@ -545,22 +545,11 @@ def test_excluded_user_still_blocked_by_group_rate_limit():
     cm = create_connection_manager([endpoint], excluded_uids=["user123"])
     route_metadata = create_route_metadata()
 
-    assert should_ratelimit_request(
-        route_metadata, "1.2.3.4", {"id": "user123"}, cm, "group1"
-    ) == {"block": False}
-    assert should_ratelimit_request(
-        route_metadata, "1.2.3.4", {"id": "user123"}, cm, "group1"
-    ) == {"block": False}
-    assert should_ratelimit_request(
-        route_metadata, "1.2.3.4", {"id": "user123"}, cm, "group1"
-    ) == {"block": False}
-    # Group rate limit still applies even for excluded users
-    assert should_ratelimit_request(
-        route_metadata, "1.2.3.4", {"id": "user123"}, cm, "group1"
-    ) == {
-        "block": True,
-        "trigger": "group",
-    }
+    # Excluded user should never be blocked, even past maxRequests, even with a group set
+    for _ in range(5):
+        assert should_ratelimit_request(
+            route_metadata, "1.2.3.4", {"id": "user123"}, cm, "group1"
+        ) == {"block": False}
 
 
 def test_rate_limits_by_group_if_user_is_not_set():