Explorar el Código

fix: rate-limit /api/v1/auth/* (SEC_REVIEW F14)

The auth route group previously attached only TokenAuthenticationMiddleware
and AuditContextMiddleware. A service-token holder could pound
GET /users/{id} (F17 enumeration), upsert-local, or upsert-oidc at
unbounded rate. Adding RateLimitMiddleware to the group caps a burst
at API_RATE_LIMIT_PER_SECOND × 2 per service-token bucket — same
limiter that gates the public ingest path.

The bucket bails out cleanly when no principal is set, so 401 paths
stay unaffected.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chiappa hace 5 días
padre
commit
98497796c9
Se han modificado 2 ficheros con 58 adiciones y 2 borrados
  1. 6 2
      api/src/App/AppFactory.php
  2. 52 0
      api/tests/Integration/Public/RateLimitTest.php

+ 6 - 2
api/src/App/AppFactory.php

@@ -53,7 +53,7 @@ use Slim\Routing\RouteCollectorProxy;
  *  - Public    /api/v1/report               TokenAuth → RateLimit → controller (kind check inside)
  *  - Admin     /api/v1/admin/{reporters,consumers,tokens}  TokenAuth → Impersonation → Rbac(Admin)
  *  - Admin     /api/v1/admin/me             TokenAuth → Impersonation → Rbac(Viewer)
- *  - Auth      /api/v1/auth/*               TokenAuth (controller checks kind=service)
+ *  - Auth      /api/v1/auth/*               TokenAuth → AuditContext → RateLimit (controller checks kind=service)
  */
 final class AppFactory
 {
@@ -155,7 +155,10 @@ final class AppFactory
         // exist to *produce* user_ids the ui can later impersonate. The
         // controller enforces kind=service on each call. Audit context is
         // attached so user.created / user.role_changed audit rows
-        // (SEC_REVIEW F5) carry source IP and request id.
+        // (SEC_REVIEW F5) carry source IP and request id. Rate limit is
+        // attached so a leaked service token cannot brute-force-enumerate
+        // users via GET /users/{id} (SEC_REVIEW F14, capping the
+        // enumeration speed flagged separately by F17).
         $app->group('/api/v1/auth', function (RouteCollectorProxy $auth) use ($container): void {
             /** @var AuthController $controller */
             $controller = $container->get(AuthController::class);
@@ -170,6 +173,7 @@ final class AppFactory
                 return $controller->getUser($request, $response, $args['id']);
             });
         })
+            ->add($rateLimit)
             ->add($auditContext)
             ->add($tokenAuth);
 

+ 52 - 0
api/tests/Integration/Public/RateLimitTest.php

@@ -89,4 +89,56 @@ final class RateLimitTest extends AppTestCase
             self::assertNotSame(429, $resp->getStatusCode());
         }
     }
+
+    /**
+     * SEC_REVIEW F14. The /api/v1/auth/* group previously had only
+     * TokenAuth, so a service-token holder could brute-force enumerate
+     * users via /users/{id} (F17) at unbounded rate, or burn unbounded
+     * upsert calls. RateLimitMiddleware is now attached; a burst against
+     * the auth endpoints must produce 429s once the bucket drains.
+     */
+    public function testAuthGetUserRouteIsRateLimited(): void
+    {
+        $service = $this->createToken(TokenKind::Service);
+        $headers = ['Authorization' => 'Bearer ' . $service];
+
+        $statuses = [];
+        for ($i = 0; $i < 20; $i++) {
+            $statuses[] = $this->request('GET', '/api/v1/auth/users/1', $headers)->getStatusCode();
+        }
+
+        $limited = count(array_filter($statuses, static fn (int $s): bool => $s === 429));
+        self::assertGreaterThan(
+            0,
+            $limited,
+            'auth/users/{id} must hit the rate-limit ceiling on burst (SEC_REVIEW F14)'
+        );
+        // Capacity is 4 (refill=2, capacity=4 from setUp); the rest must 429.
+        self::assertSame(16, $limited, 'remainder rate-limited');
+    }
+
+    /**
+     * SEC_REVIEW F14. upsertLocal/upsertOidc are also gated by the same
+     * rate limit — a leaked service token cannot pound them at unbounded
+     * rate to amplify other findings (e.g. burning audit rows, retrying
+     * upsert combinations).
+     */
+    public function testAuthUpsertLocalRouteIsRateLimited(): void
+    {
+        $service = $this->createToken(TokenKind::Service);
+        $headers = [
+            'Authorization' => 'Bearer ' . $service,
+            'Content-Type' => 'application/json',
+        ];
+        $body = json_encode(['username' => 'admin']) ?: null;
+
+        $statuses = [];
+        for ($i = 0; $i < 20; $i++) {
+            $statuses[] = $this->request('POST', '/api/v1/auth/users/upsert-local', $headers, $body)
+                ->getStatusCode();
+        }
+
+        $limited = count(array_filter($statuses, static fn (int $s): bool => $s === 429));
+        self::assertGreaterThan(0, $limited, 'upsert-local must rate-limit');
+    }
 }