feat: add configurable per-user rate limiting for review submissions

src/config.default.toml

@@ -45,3 +45,10 @@ client_id = "aiod-api"  # a private client, used by the backend
 client_id_swagger = "aiod-api-swagger"  # a public client, used by the Swagger Frontend
 openid_connect_url = "http://localhost/aiod-auth/realms/aiod/.well-known/openid-configuration"
 scopes = "openid profile roles"
+
+# Rate limiting configuration
+[rate_limits]
+# Maximum number of review submissions per user within the time window
+max_submissions_per_user = 10
+# Time window in seconds (default: 3600 = 1 hour)
+submission_window_seconds = 3600

src/config.py

@@ -75,3 +75,4 @@ def log_configuration():
 KEYCLOAK_CONFIG = CONFIG.get("keycloak", {})
 DEV_CONFIG = CONFIG.get("dev", {})
 REQUEST_TIMEOUT = CONFIG.get("dev", {}).get("request_timeout", None)
+RATE_LIMIT_CONFIG = CONFIG.get("rate_limits", {})

src/dependencies/rate_limiter.py

@@ -0,0 +1,73 @@
+"""
+Per-user rate limiting for review submission requests (issue #663).
+
+Uses an in-memory rolling window to track submissions per user.
+Connectors (users with platform_* roles) bypass rate limiting as they use
+a separate workflow for systematic data migrations.
+"""
+
+import datetime
+import logging
+from collections import defaultdict, deque
+
+from fastapi import Depends, HTTPException, status
+
+from authentication import KeycloakUser, get_user_or_raise
+from config import RATE_LIMIT_CONFIG
+
+logger = logging.getLogger(__name__)
+
+# In-memory store: maps user subject_identifier -> deque of submission timestamps
+_submission_timestamps: dict[str, deque[datetime.datetime]] = defaultdict(deque)
+
+MAX_SUBMISSIONS: int = RATE_LIMIT_CONFIG.get("max_submissions_per_user", 10)
+WINDOW_SECONDS: int = RATE_LIMIT_CONFIG.get("submission_window_seconds", 3600)
+
+
+def _clean_expired_entries(user_id: str) -> None:
+    """Remove timestamps that fall outside the current rolling window."""
+    cutoff = datetime.datetime.now(datetime.timezone.utc) - datetime.timedelta(
+        seconds=WINDOW_SECONDS
+    )
+    timestamps = _submission_timestamps[user_id]
+    while timestamps and timestamps[0] < cutoff:
+        timestamps.popleft()
+
+
+def check_submission_rate_limit(
+    user: KeycloakUser = Depends(get_user_or_raise),
+) -> None:
+    """
+    FastAPI dependency that enforces per-user rate limiting on review submissions.
+
+    Connectors (platform roles) are exempt — they use the connector workflow
+    designed for systematic uploads (see issue #663).
+
+    Raises:
+        HTTPException 429 if the user has exceeded max_submissions_per_user
+        within the submission_window_seconds rolling window.
+    """
+    if user.is_connector:
+        return  # connectors bypass rate limiting
+
+    user_id = user._subject_identifier
+    _clean_expired_entries(user_id)
+
+    timestamps = _submission_timestamps[user_id]
+    if len(timestamps) >= MAX_SUBMISSIONS:
+        window_minutes = WINDOW_SECONDS // 60
+        logger.warning(
+            f"Rate limit exceeded for user {user.name!r}: "
+            f"{len(timestamps)}/{MAX_SUBMISSIONS} submissions in {window_minutes} min window."
+        )
+        raise HTTPException(
+            status_code=status.HTTP_429_TOO_MANY_REQUESTS,
+            detail=(
+                f"Rate limit exceeded: you may submit at most {MAX_SUBMISSIONS} "
+                f"review requests per {window_minutes} minutes. "
+                "Please try again later."
+            ),
+        )
+
+    # Record this submission timestamp
+    timestamps.append(datetime.datetime.now(datetime.timezone.utc))

src/routers/review_router.py

@@ -23,6 +23,7 @@
 from database.model.concept.concept import AIoDConcept
 from routers.helper_functions import get_router_by_type
 from database.model.helper_functions import get_asset_type_by_abbreviation
+from dependencies.rate_limiter import check_submission_rate_limit
 from versioning import Version
 
 
@@ -141,6 +142,7 @@ def get_submission(
 def _submit_resource(
     submission: SubmissionCreate,
     user: KeycloakUser = Depends(get_user_or_raise),
+    _rate_check: None = Depends(check_submission_rate_limit),
 ):
     id_to_type = {
         identifier: get_asset_type_by_abbreviation().get(identifier.split("_")[0])

src/tests/test_review_rate_limit.py

@@ -0,0 +1,120 @@
+"""
+Tests for per-user rate limiting on review submission requests (issue #663).
+"""
+
+import datetime
+from collections import deque
+
+import pytest
+from fastapi import HTTPException
+
+from authentication import KeycloakUser
+from dependencies.rate_limiter import (
+    MAX_SUBMISSIONS,
+    WINDOW_SECONDS,
+    _submission_timestamps,
+    check_submission_rate_limit,
+)
+
+
+def _make_user(
+    subject_id: str = "user-001",
+    name: str = "testuser",
+    roles: set[str] | None = None,
+) -> KeycloakUser:
+    if roles is None:
+        roles = {"offline_access", "uma_authorization", "default-roles-aiod"}
+    return KeycloakUser(name=name, roles=roles, _subject_identifier=subject_id)
+
+
+@pytest.fixture(autouse=True)
+def _clear_rate_limit_state():
+    """Ensure a clean rate limiter state for every test."""
+    _submission_timestamps.clear()
+    yield
+    _submission_timestamps.clear()
+
+
+class TestSubmissionRateLimit:
+
+    def test_allows_submissions_within_limit(self):
+        """Submissions at or below the limit should pass."""
+        user = _make_user()
+        for _ in range(MAX_SUBMISSIONS):
+            check_submission_rate_limit(user)
+        assert len(_submission_timestamps[user._subject_identifier]) == MAX_SUBMISSIONS
+
+    def test_rejects_submission_over_limit(self):
+        """One submission beyond the limit must raise HTTP 429."""
+        user = _make_user()
+        for _ in range(MAX_SUBMISSIONS):
+            check_submission_rate_limit(user)
+
+        with pytest.raises(HTTPException) as exc_info:
+            check_submission_rate_limit(user)
+        assert exc_info.value.status_code == 429
+        assert "Rate limit exceeded" in exc_info.value.detail
+
+    def test_allows_after_window_expires(self):
+        """Submissions from outside the window should be cleaned and new ones allowed."""
+        user = _make_user()
+        expired = datetime.datetime.now(datetime.timezone.utc) - datetime.timedelta(
+            seconds=WINDOW_SECONDS + 1
+        )
+        _submission_timestamps[user._subject_identifier] = deque(
+            [expired] * MAX_SUBMISSIONS
+        )
+        # All expired — should succeed
+        check_submission_rate_limit(user)
+        assert len(_submission_timestamps[user._subject_identifier]) == 1
+
+    def test_connector_bypasses_rate_limit(self):
+        """Users with platform_* roles (connectors) are never rate-limited."""
+        connector = _make_user(
+            subject_id="connector-001",
+            roles={"platform_huggingface", "default-roles-aiod"},
+        )
+        for _ in range(MAX_SUBMISSIONS + 5):
+            check_submission_rate_limit(connector)
+        assert "connector-001" not in _submission_timestamps
+
+    def test_independent_counters_per_user(self):
+        """Each user has an independent rate limit counter."""
+        alice = _make_user(subject_id="alice", name="alice")
+        bob = _make_user(subject_id="bob", name="bob")
+
+        for _ in range(MAX_SUBMISSIONS):
+            check_submission_rate_limit(alice)
+
+        # Bob should still be free
+        check_submission_rate_limit(bob)
+        assert len(_submission_timestamps["bob"]) == 1
+
+        # Alice should be blocked
+        with pytest.raises(HTTPException) as exc_info:
+            check_submission_rate_limit(alice)
+        assert exc_info.value.status_code == 429
+
+    def test_partial_expiry_keeps_recent_entries(self):
+        """Expired entries are cleaned but recent ones are preserved."""
+        user = _make_user()
+        now = datetime.datetime.now(datetime.timezone.utc)
+        expired = now - datetime.timedelta(seconds=WINDOW_SECONDS + 1)
+        recent = now - datetime.timedelta(seconds=60)
+        # 5 expired + 4 recent = 9 — under the limit
+        _submission_timestamps[user._subject_identifier] = deque(
+            [expired] * 5 + [recent] * 4
+        )
+        check_submission_rate_limit(user)
+        # 5 expired cleaned, 4 recent + 1 new = 5
+        assert len(_submission_timestamps[user._subject_identifier]) == 5
+
+    def test_error_message_includes_limits(self):
+        """The 429 detail must include the configured limit and window."""
+        user = _make_user()
+        for _ in range(MAX_SUBMISSIONS):
+            check_submission_rate_limit(user)
+        with pytest.raises(HTTPException) as exc_info:
+            check_submission_rate_limit(user)
+        assert str(MAX_SUBMISSIONS) in exc_info.value.detail
+        assert str(WINDOW_SECONDS // 60) in exc_info.value.detail