Skip to content

Commit b9e0b95

Browse files
Copilotazvoleff
andauthored
Fix active rate limits endpoint and add targeted reset capability (#81)
* Initial plan * Fix active rate limits endpoint and add reset by ID functionality - Fix get_current_rate_limits() to only return limits with count > 0 - Add reset_rate_limit_by_key() helper function to reset individual limits - Add new endpoint POST /api/v1/rate-limit/reset/<identifier> for targeted resets - Add comprehensive tests for new functionality - All code passes ruff linting and formatting Co-authored-by: azvoleff <107753+azvoleff@users.noreply.github.com> * Address code review feedback - Fix Python 3.10+ union syntax (list | tuple) to use isinstance(keys, (list, tuple)) - Clarify comment about filtering limits with count > 0 - Move jwt import to top of test file per PEP 8 - All ruff checks pass Co-authored-by: azvoleff <107753+azvoleff@users.noreply.github.com> * Use fixture instead of JWT decoding in tests - Replace manual JWT decoding with regular_user fixture for better security - Remove jwt import as it's no longer needed - Avoid disabling signature verification in tests - Use project's standard fixtures for consistent test patterns Co-authored-by: azvoleff <107753+azvoleff@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: azvoleff <107753+azvoleff@users.noreply.github.com>
1 parent 19a20f1 commit b9e0b95

File tree

3 files changed

+326
-2
lines changed

3 files changed

+326
-2
lines changed

gefapi/routes/api/v1/admin.py

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,61 @@ def get_rate_limit_status():
100100
return jsonify({"error": "Failed to retrieve rate limiting status"}), 500
101101

102102

103+
@endpoints.route("/rate-limit/reset/<string:identifier>", methods=["POST"])
104+
@jwt_required()
105+
def reset_rate_limit_by_identifier(identifier):
106+
"""
107+
Reset a specific rate limit by its identifier.
108+
109+
**Access**: Restricted to users with `role: "SUPERADMIN"`
110+
**Purpose**: Clears rate limit counters for a specific user or IP address
111+
112+
**Path Parameters**:
113+
- `identifier`: The rate limit identifier (e.g., "user:123", "ip:192.168.1.1",
114+
"auth:hash:ip")
115+
116+
**Success Response Schema**:
117+
```json
118+
{
119+
"message": "Rate limit reset for identifier: user:123"
120+
}
121+
```
122+
123+
**Use Cases**:
124+
- Clear rate limit for specific user who was legitimately rate limited
125+
- Remove rate limit for specific IP address
126+
- Targeted rate limit management without affecting all users
127+
128+
**Error Responses**:
129+
- `403 Forbidden`: User does not have SUPERADMIN privileges
130+
- `401 Unauthorized`: Valid JWT token required
131+
- `404 Not Found`: No rate limit found for the specified identifier
132+
- `500 Internal Server Error`: Failed to reset rate limit
133+
"""
134+
current_user_id = get_jwt_identity()
135+
user = UserService.get_user(current_user_id)
136+
137+
if not user or user.role != "SUPERADMIN":
138+
return jsonify({"msg": "Superadmin access required"}), 403
139+
140+
try:
141+
from gefapi.utils.rate_limiting import reset_rate_limit_by_key
142+
143+
success = reset_rate_limit_by_key(identifier)
144+
145+
if success:
146+
return jsonify(
147+
{"message": f"Rate limit reset for identifier: {identifier}"}
148+
), 200
149+
return jsonify(
150+
{"error": f"No rate limit found for identifier: {identifier}"}
151+
), 404
152+
153+
except Exception as e:
154+
app.logger.error(f"Failed to reset rate limit for {identifier}: {e}")
155+
return jsonify({"error": "Failed to reset rate limit"}), 500
156+
157+
103158
@endpoints.route("/rate-limit/reset", methods=["POST"])
104159
@jwt_required()
105160
def reset_rate_limits():

gefapi/utils/rate_limiting.py

Lines changed: 85 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -572,7 +572,7 @@ def get_current_rate_limits():
572572
# Redis storage - get all keys matching rate limit patterns
573573
pattern = "LIMITER/*"
574574
keys = storage.storage.keys(pattern)
575-
if isinstance(keys, list | tuple):
575+
if isinstance(keys, (list, tuple)):
576576
rate_limit_keys = [
577577
key.decode() if isinstance(key, bytes) else key for key in keys
578578
]
@@ -712,7 +712,8 @@ def get_current_rate_limits():
712712
limit_info["identifier"] = rate_key
713713

714714
# Only include limits that have a current count > 0 (actively limiting)
715-
if current_count_numeric is not None:
715+
# This filters out keys that are tracked but have no current activity
716+
if current_count_numeric is not None and current_count_numeric > 0:
716717
active_limits.append(limit_info)
717718

718719
except Exception as e:
@@ -736,6 +737,88 @@ def get_current_rate_limits():
736737
}
737738

738739

740+
def reset_rate_limit_by_key(limit_key: str) -> bool:
741+
"""
742+
Reset a specific rate limit by its key identifier.
743+
744+
Args:
745+
limit_key: The rate limit key to reset (e.g., "user:123", "ip:192.168.1.1")
746+
747+
Returns:
748+
True if the rate limit was reset successfully, False otherwise.
749+
"""
750+
try:
751+
from gefapi import limiter
752+
753+
if not limiter.enabled or not RateLimitConfig.is_enabled():
754+
logger.warning("Rate limiting is disabled, cannot reset rate limit")
755+
return False
756+
757+
storage = limiter._storage
758+
keys_deleted = 0
759+
760+
# Try to find and delete all keys matching this rate limit identifier
761+
try:
762+
# For Redis storage
763+
if hasattr(storage, "storage") and hasattr(storage.storage, "keys"):
764+
# Redis storage - find all keys matching this identifier
765+
pattern = f"LIMITER/{limit_key}/*"
766+
keys = storage.storage.keys(pattern)
767+
if isinstance(keys, (list, tuple)):
768+
for key in keys:
769+
key_str = key.decode() if isinstance(key, bytes) else key
770+
try:
771+
storage.storage.delete(key)
772+
keys_deleted += 1
773+
logger.info(f"Deleted rate limit key: {key_str}")
774+
except Exception as e:
775+
logger.warning(f"Failed to delete key {key_str}: {e}")
776+
777+
elif hasattr(storage, "storage") and hasattr(storage.storage, "scan_iter"):
778+
# Redis storage with scan_iter method
779+
pattern = f"LIMITER/{limit_key}/*"
780+
for key in storage.storage.scan_iter(match=pattern):
781+
key_str = key.decode() if isinstance(key, bytes) else key
782+
try:
783+
storage.storage.delete(key)
784+
keys_deleted += 1
785+
logger.info(f"Deleted rate limit key: {key_str}")
786+
except Exception as e:
787+
logger.warning(f"Failed to delete key {key_str}: {e}")
788+
789+
elif hasattr(storage, "storage") and isinstance(storage.storage, dict):
790+
# Memory storage - direct dictionary access
791+
keys_to_delete = [
792+
k for k in storage.storage if k.startswith(f"LIMITER/{limit_key}/")
793+
]
794+
for key in keys_to_delete:
795+
try:
796+
del storage.storage[key]
797+
keys_deleted += 1
798+
logger.info(f"Deleted rate limit key: {key}")
799+
except Exception as e:
800+
logger.warning(f"Failed to delete key {key}: {e}")
801+
else:
802+
logger.warning("Unable to determine storage type for rate limit reset")
803+
return False
804+
805+
except Exception as e:
806+
logger.error(f"Failed to reset rate limit for key {limit_key}: {e}")
807+
return False
808+
809+
if keys_deleted > 0:
810+
logger.info(
811+
f"Successfully reset {keys_deleted} rate limit keys for {limit_key}"
812+
)
813+
return True
814+
logger.warning(f"No rate limit keys found for {limit_key}")
815+
return False
816+
817+
except Exception as e:
818+
logger.error(f"Failed to reset rate limit by key: {e}")
819+
return False
820+
821+
739822
def reconfigure_limiter_for_testing():
740823
"""
741824
Reconfigure the Flask-Limiter instance for testing.
Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
"""Tests for rate limit reset by identifier functionality"""
2+
3+
4+
class TestRateLimitResetByIdentifier:
5+
"""Test resetting specific rate limits by identifier"""
6+
7+
def test_reset_rate_limit_by_identifier_superadmin_access(
8+
self,
9+
client,
10+
auth_headers_user,
11+
auth_headers_superadmin,
12+
reset_rate_limits,
13+
regular_user,
14+
):
15+
"""Test that superadmin can reset a specific rate limit by identifier"""
16+
# First, reset all rate limits to start fresh
17+
reset_rate_limits()
18+
19+
# Trigger rate limit for a specific user
20+
responses = []
21+
for i in range(100): # Make many requests to trigger rate limiting
22+
response = client.get("/api/v1/user/me", headers=auth_headers_user)
23+
responses.append(response)
24+
if response.status_code == 429:
25+
break
26+
27+
# Check if user was rate limited
28+
rate_limited = any(r.status_code == 429 for r in responses)
29+
if not rate_limited:
30+
# If not rate limited, skip this part of the test
31+
return
32+
33+
# Get the user ID from the fixture
34+
# The regular_user fixture provides the user object with known ID
35+
user_id = regular_user.id
36+
37+
# Reset the rate limit for this specific user
38+
response = client.post(
39+
f"/api/v1/rate-limit/reset/user:{user_id}",
40+
headers=auth_headers_superadmin,
41+
)
42+
assert response.status_code in [
43+
200,
44+
404,
45+
], f"Expected 200 or 404, got {response.status_code}: {response.json}"
46+
47+
# If reset was successful, verify the response
48+
if response.status_code == 200:
49+
data = response.json
50+
assert "message" in data
51+
assert str(user_id) in data["message"]
52+
53+
def test_reset_rate_limit_by_identifier_admin_forbidden(
54+
self, client, auth_headers_admin
55+
):
56+
"""Test that admin cannot reset rate limits by identifier"""
57+
response = client.post(
58+
"/api/v1/rate-limit/reset/user:123", headers=auth_headers_admin
59+
)
60+
assert response.status_code == 403
61+
assert "Superadmin access required" in response.json["msg"]
62+
63+
def test_reset_rate_limit_by_identifier_user_forbidden(
64+
self, client, auth_headers_user
65+
):
66+
"""Test that regular user cannot reset rate limits by identifier"""
67+
response = client.post(
68+
"/api/v1/rate-limit/reset/user:123", headers=auth_headers_user
69+
)
70+
assert response.status_code == 403
71+
assert "Superadmin access required" in response.json["msg"]
72+
73+
def test_reset_rate_limit_by_identifier_no_auth_forbidden(self, client):
74+
"""Test that unauthenticated request cannot reset rate limits by identifier"""
75+
response = client.post("/api/v1/rate-limit/reset/user:123")
76+
assert response.status_code == 401 # JWT required
77+
78+
def test_reset_rate_limit_by_ip(self, client, auth_headers_superadmin):
79+
"""Test resetting rate limit by IP address"""
80+
# Try to reset a rate limit by IP
81+
response = client.post(
82+
"/api/v1/rate-limit/reset/ip:192.168.1.100", headers=auth_headers_superadmin
83+
)
84+
# Should either succeed (200) or not find the rate limit (404)
85+
assert response.status_code in [200, 404]
86+
87+
if response.status_code == 200:
88+
data = response.json
89+
assert "message" in data
90+
assert "192.168.1.100" in data["message"]
91+
else:
92+
# 404 is acceptable if no rate limit exists for this IP
93+
data = response.json
94+
assert "error" in data
95+
96+
def test_reset_rate_limit_returns_404_for_nonexistent(
97+
self, client, auth_headers_superadmin, reset_rate_limits
98+
):
99+
"""Test that resetting a nonexistent rate limit returns 404"""
100+
# First, reset all rate limits to ensure clean state
101+
reset_rate_limits()
102+
103+
# Try to reset a rate limit that doesn't exist
104+
response = client.post(
105+
"/api/v1/rate-limit/reset/user:nonexistent-user-id",
106+
headers=auth_headers_superadmin,
107+
)
108+
assert response.status_code == 404
109+
data = response.json
110+
assert "error" in data
111+
assert "No rate limit found" in data["error"]
112+
113+
def test_reset_rate_limit_by_auth_identifier(self, client, auth_headers_superadmin):
114+
"""Test resetting rate limit by auth identifier"""
115+
# Try to reset an auth-type rate limit
116+
response = client.post(
117+
"/api/v1/rate-limit/reset/auth:somehash:someip",
118+
headers=auth_headers_superadmin,
119+
)
120+
# Should either succeed (200) or not find the rate limit (404)
121+
assert response.status_code in [200, 404]
122+
123+
124+
class TestRateLimitStatusImprovements:
125+
"""Test improvements to rate limit status endpoint"""
126+
127+
def test_rate_limit_status_shows_only_active_limits(
128+
self, client, auth_headers_user, auth_headers_superadmin, reset_rate_limits
129+
):
130+
"""Test that rate limit status only shows limits with count > 0"""
131+
# Reset all limits first
132+
reset_rate_limits()
133+
134+
# Make a few requests (but not enough to trigger rate limiting)
135+
for i in range(3):
136+
client.get("/api/v1/user/me", headers=auth_headers_user)
137+
138+
# Check rate limit status
139+
response = client.get(
140+
"/api/v1/rate-limit/status", headers=auth_headers_superadmin
141+
)
142+
assert response.status_code == 200
143+
144+
data = response.json["data"]
145+
if data["enabled"]:
146+
# All active limits should have current_count > 0
147+
for limit in data["active_limits"]:
148+
assert "current_count" in limit
149+
# Current count should be greater than 0 for "active" limits
150+
if isinstance(limit["current_count"], (int, float)):
151+
assert limit["current_count"] > 0
152+
153+
def test_rate_limit_status_includes_user_info_for_user_limits(
154+
self, client, auth_headers_user, auth_headers_superadmin, reset_rate_limits
155+
):
156+
"""Test that user-type limits include user information"""
157+
# Reset and trigger some user requests
158+
reset_rate_limits()
159+
160+
# Make requests to trigger rate limit tracking
161+
for i in range(5):
162+
client.get("/api/v1/user/me", headers=auth_headers_user)
163+
164+
# Check rate limit status
165+
response = client.get(
166+
"/api/v1/rate-limit/status", headers=auth_headers_superadmin
167+
)
168+
assert response.status_code == 200
169+
170+
data = response.json["data"]
171+
if data["enabled"] and data["active_limits"]:
172+
# Find any user-type limits
173+
user_limits = [
174+
limit for limit in data["active_limits"] if limit["type"] == "user"
175+
]
176+
177+
# If there are user limits, they should have user_info
178+
for limit in user_limits:
179+
assert "type" in limit
180+
assert limit["type"] == "user"
181+
# User info may or may not be present depending on whether
182+
# the user lookup succeeded
183+
if limit.get("user_info"):
184+
assert "id" in limit["user_info"]
185+
assert "email" in limit["user_info"]
186+
assert "role" in limit["user_info"]

0 commit comments

Comments
 (0)