Fix: same-origin bypass let blocked IPs through on /api/

_handle_api checked _is_same_origin() before any of the IP-prefix, global
(rl:ip:<ip>:blocked), and API-specific (rl:api:ip:<ip>:blocked) block keys —
short-circuiting straight to get_response() on a match. Since Origin/Referer
are entirely client-controlled and trivially spoofable, any caller could
defeat every /api/ block and counter (including an operator-set global block)
by simply sending Origin: https://<sapl-host>.

Reorder the checks so IP-prefix/global/API block lookups always run first;
the same-origin bypass now only exempts legitimate same-origin polling from
quota and per-minute rate-limit accounting, never from an active block.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

sapl/middleware/ratelimit.py

@@ -6,14 +6,19 @@ Decision flow (per request):
     -1. IP in rl:ip_prefix:blocked (prefix match)? → 429
         (universal — applies to authenticated users too, like the UA bot checks)
   /api/ paths — handled by _handle_api:
-    0a. OPTIONS?            → pass (CORS preflight must never be blocked)
-    0b. Same-origin?        → pass (SAPL's own browser polling)
-    0c. rl:ip:<ip>:blocked? → 429  (global block also covers /api/)
+    0a. OPTIONS?                → pass (CORS preflight must never be blocked)
+    0b. rl:ip_prefix:blocked?   → 429  (prefix match; see -1 above)
+    0c. rl:ip:<ip>:blocked?     → 429  (global block also covers /api/)
     0d. rl:api:ip:<ip>:blocked? → 429  (API-only block)
-    0e. Daily/weekly quota exceeded? → 429
-    0f. Anon + API threshold exceeded? → SET rl:api:ip:<ip>:blocked, 429
+        Block checks 0b-0d always run before the same-origin check below —
+        Origin/Referer are client-controlled and trivially spoofable, so they
+        must never be able to override an already-made block decision.
+    0e. Same-origin?            → pass, skipping 0f-0g only (SAPL's own polling
+        is exempt from quota/rate-limit accounting, never from active blocks)
+    0f. Daily/weekly quota exceeded? → 429
+    0g. Anon + API threshold exceeded? → SET rl:api:ip:<ip>:blocked, 429
         (never writes rl:ip:<ip>:blocked)
-    0g. Auth: falls through to _evaluate (per-user counter)
+    0h. Auth: falls through to _evaluate (per-user counter)
   Non-/api/ paths:
   1. Known bot UA?          → 429  (Python list — substring match)
   1b. Redis UA deny list?   → 429  (runtime SET — token hash match, refreshed every 60 s)
@@ -448,13 +453,12 @@ class RateLimitMiddleware:
         if request.method == 'OPTIONS':
             return self.get_response(request)
 
-        # 2. Same-origin (SAPL's own polling) — no counter, no block
-        if self.api_same_origin_bypass and _is_same_origin(request):
-            return self.get_response(request)
-
         ip = get_client_ip(request)
 
-        # 3a. IP-prefix block — operator-curated deny list, applies to everyone
+        # 2. IP-prefix block — operator-curated deny list, applies to everyone.
+        #    Block checks (2-4) must run before the same-origin bypass below:
+        #    Origin/Referer are client-controlled and trivially spoofable, so
+        #    they must never be able to override an already-made block decision.
         if self._is_ip_prefix_blocked(ip):
             logger.warning(
                 'api_rate_limit_block reason=ip_prefix_blocked ip=%s path=%s user_agent=%s',
@@ -481,7 +485,13 @@ class RateLimitMiddleware:
             self._inc_block_metric('api_ip_blocked')
             return self._api_block_response('api_ip_blocked')
 
-        # 5. Daily/weekly quota (existing logic, preserved)
+        # 5. Same-origin (SAPL's own polling) — exempt from quota/rate-limit
+        #    accounting only (steps 6-7 below). Must come after the block
+        #    checks above, never before them.
+        if self.api_same_origin_bypass and _is_same_origin(request):
+            return self.get_response(request)
+
+        # 6. Daily/weekly quota (existing logic, preserved)
         exceeded = self._check_api_quota(request)
         if exceeded:
             logger.warning(
@@ -495,7 +505,7 @@ class RateLimitMiddleware:
             response['X-RateLimit-Reason'] = f'quota_{exceeded}'
             return response
 
-        # 6. Per-minute rate limit — 60/min for all callers (anon and auth).
+        # 7. Per-minute rate limit — 60/min for all callers (anon and auth).
         #    Auth is not exempt: authenticating must not bypass this cap.
         #    Writes rl:api:ip:<ip>:blocked only — never rl:ip:<ip>:blocked.
         if self.api_rate_limit_enabled:

sapl/middleware/test_ratelimiter.py

@@ -647,6 +647,76 @@ def test_api_malicious_origin_is_not_same_origin():
     mw._incr_with_ttl.assert_called_once()
 
 
+# Same-origin headers a spoofing client can trivially forge — Origin/Referer
+# carry no authentication, so they must never override an active block.
+_SAME_ORIGIN_META = {
+    'SERVER_NAME': 'sapl.example.com',
+    'SERVER_PORT': '80',
+    'HTTP_HOST': 'sapl.example.com',
+    'HTTP_ORIGIN': 'https://sapl.example.com',
+}
+
+
+def test_api_same_origin_does_not_bypass_global_ip_block():
+    mw, mock_cache = _make_middleware()
+    ip = '1.2.3.4'
+    mock_cache.get.side_effect = lambda key: 1 if key == RL_IP_BLOCKED.format(ip=ip) else None
+    mw._check_api_quota = MagicMock(return_value=None)
+    mw._incr_with_ttl = MagicMock()
+    request = _api_req(ip=ip, extra_meta=_SAME_ORIGIN_META)
+
+    response = mw(request)
+
+    mw.get_response.assert_not_called()
+    assert response.status_code == 429
+    assert response['X-RateLimit-Reason'] == 'global_ip_blocked'
+
+
+def test_api_same_origin_does_not_bypass_api_ip_block():
+    mw, mock_cache = _make_middleware()
+    ip = '1.2.3.4'
+    blocked_key = RL_API_IP_BLOCKED.format(ns=_NAMESPACE, ip=ip)
+    mock_cache.get.side_effect = lambda key: 1 if key == blocked_key else None
+    mw._check_api_quota = MagicMock(return_value=None)
+    mw._incr_with_ttl = MagicMock()
+    request = _api_req(ip=ip, extra_meta=_SAME_ORIGIN_META)
+
+    response = mw(request)
+
+    mw.get_response.assert_not_called()
+    assert response.status_code == 429
+    assert response['X-RateLimit-Reason'] == 'api_ip_blocked'
+
+
+def test_api_same_origin_does_not_bypass_ip_prefix_block(_seed_prefix_blocklist):
+    mw, _ = _make_middleware()
+    _seed_prefix_blocklist(['1.2.3'])
+    mw._check_api_quota = MagicMock(return_value=None)
+    mw._incr_with_ttl = MagicMock()
+    request = _api_req(ip='1.2.3.4', extra_meta=_SAME_ORIGIN_META)
+
+    response = mw(request)
+
+    mw.get_response.assert_not_called()
+    assert response.status_code == 429
+    assert response['X-RateLimit-Reason'] == 'ip_prefix_blocked'
+
+
+def test_api_same_origin_still_skips_quota_and_rate_limit_when_not_blocked():
+    """Legitimate same-origin polling keeps its original exemption from accounting."""
+    mw, _ = _make_middleware()
+    mw._check_api_quota = MagicMock(return_value=None)
+    mw._incr_with_ttl = MagicMock()
+    request = _api_req(extra_meta=_SAME_ORIGIN_META)
+
+    response = mw(request)
+
+    mw.get_response.assert_called_once_with(request)
+    mw._check_api_quota.assert_not_called()
+    mw._incr_with_ttl.assert_not_called()
+    assert response.status_code == 200
+
+
 # ---------------------------------------------------------------------------
 # _handle_api — Check 3a: IP-prefix block (operator-curated deny list)
 # ---------------------------------------------------------------------------