fix: normalize URI by stripping query params in rate limiter (Closes #167) (#180)

lib/Middleware/RateLimitMiddleware.php

@@ -77,7 +77,10 @@ class RateLimitMiddleware extends Middleware {
         $uri = $this->request->getRequestUri();
         [$maxRequests, $windowSeconds] = $this->getLimitForUri($uri);
 
-        $cacheKey = 'rl_' . $user->getUID() . '_' . md5($uri);
+        // Normalize URI by stripping query parameters to prevent bypass
+        // via appending varying dummy params (e.g., ?_=1, ?_=2)
+        $normalizedUri = strtok($uri, '?') ?: $uri;
+        $cacheKey = 'rl_' . $user->getUID() . '_' . md5($normalizedUri);
         $current = $this->cache->get($cacheKey);
 
         if ($current === null) {

tests/Unit/RateLimitMiddlewareTest.php

@@ -209,6 +209,47 @@ class RateLimitMiddlewareTest extends TestCase {
         $middleware->beforeController(new \stdClass(), 'index');
     }
 
+    // -- Query parameter normalization --
+
+    public function testSameEndpointWithDifferentQueryParamsSharesBucket(): void {
+        $middleware = $this->createMiddleware();
+        $this->userSession->method('getUser')->willReturn($this->user);
+
+        // Two requests to the same path but with different query parameters
+        // should share the same rate limit bucket
+        $this->request->method('getRequestUri')
+            ->willReturn('/apps/mitgliederverwaltung/api/members?_=12345');
+
+        $this->cache->method('get')->willReturn(null);
+        $this->cache->expects($this->once())
+            ->method('set')
+            ->with(
+                // The cache key should be based on path without query params
+                $this->equalTo('rl_testuser_' . md5('/apps/mitgliederverwaltung/api/members')),
+                1,
+                60
+            );
+
+        $middleware->beforeController(new \stdClass(), 'index');
+    }
+
+    public function testQueryParamBypassBlocked(): void {
+        $middleware = $this->createMiddleware();
+        $this->userSession->method('getUser')->willReturn($this->user);
+
+        // Request with random query param should still be rate limited
+        $this->request->method('getRequestUri')
+            ->willReturn('/apps/mitgliederverwaltung/api/export/csv?_=99999');
+
+        // At limit for export (10)
+        $this->cache->method('get')->willReturn(10);
+
+        $this->expectException(\RuntimeException::class);
+        $this->expectExceptionMessage('Rate limit exceeded');
+
+        $middleware->beforeController(new \stdClass(), 'export');
+    }
+
     // -- afterException tests --
 
     public function testAfterExceptionReturns429ForRateLimitExceeded(): void {