From b022d68d54badf8fbfddd064e3a030476cdd103d Mon Sep 17 00:00:00 2001 From: Dan Lemon Date: Wed, 1 Jul 2026 08:31:52 +0200 Subject: [PATCH] feat: rate-limit unauthenticated public API routes --- app/Providers/AppServiceProvider.php | 35 ++++++ config/polydock.php | 1 + routes/api.php | 14 ++- tests/Feature/Api/InstanceHealthApiTest.php | 67 ++++++++++++ .../Api/RegisterControllerTest.php | 100 ++++++++++++++++++ 5 files changed, 213 insertions(+), 4 deletions(-) diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index 9a6fac93..79ba1021 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -11,9 +11,11 @@ use Dedoc\Scramble\Scramble; use Dedoc\Scramble\Support\Generator\OpenApi; use Dedoc\Scramble\Support\Generator\SecurityScheme; +use Illuminate\Cache\RateLimiting\Limit; use Illuminate\Console\Events\CommandStarting; use Illuminate\Contracts\Auth\Authenticatable; use Illuminate\Contracts\Console\Kernel; +use Illuminate\Http\Request; use Illuminate\Queue\Failed\DatabaseFailedJobProvider; use Illuminate\Queue\Failed\DynamoDbFailedJobProvider; use Illuminate\Queue\Failed\FileFailedJobProvider; @@ -25,6 +27,7 @@ use Illuminate\Support\Facades\Event; use Illuminate\Support\Facades\Gate; use Illuminate\Support\Facades\Log; +use Illuminate\Support\Facades\RateLimiter; use Illuminate\Support\ServiceProvider; class AppServiceProvider extends ServiceProvider @@ -103,6 +106,38 @@ public function register(): void */ public function boot(): void { + // Named rate limiters for the unauthenticated public API routes. Named + // limiters key on the limiter name + IP, so each route has its own + // counter — unlike anonymous `throttle:N,1`, whose signature is + // sha1(domain|ip) and is therefore shared across every route per IP. + // Trusted internal callers (e.g. MoaD) bypass the public throttles. + $isTrusted = fn (Request $request) => in_array($request->ip(), config('polydock.trusted_ips', []), true); + + // Key on IP, not UUID: these limits exist to blunt enumeration, so a + // per-UUID bucket (one fresh budget per guessed UUID) would defeat them. + RateLimiter::for('register', fn (Request $request) => $isTrusted($request) + ? Limit::none() + : Limit::perMinute(10)->by($request->ip())); + + RateLimiter::for('public-read', fn (Request $request) => $isTrusted($request) + ? Limit::none() + : Limit::perMinute(60)->by($request->ip())); + + RateLimiter::for('instance-health', function (Request $request) use ($isTrusted) { + if ($isTrusted($request)) { + return Limit::none(); + } + + // A valid health token isn't guessing, so let it through unthrottled. + $expectedToken = config('polydock.health_token'); + $suppliedToken = $request->query('token'); + if (! empty($expectedToken) && is_string($suppliedToken) && hash_equals((string) $expectedToken, $suppliedToken)) { + return Limit::none(); + } + + return Limit::perMinute(120)->by($request->ip()); + }); + Gate::define('viewApiDocs', fn (?Authenticatable $user) => true); Scramble::configure()->expose( diff --git a/config/polydock.php b/config/polydock.php index 9a760ccb..6d7931cf 100644 --- a/config/polydock.php +++ b/config/polydock.php @@ -52,6 +52,7 @@ return [ 'health_token' => env('POLYDOCK_HEALTH_TOKEN'), + 'trusted_ips' => array_filter(array_map('trim', explode(',', (string) env('POLYDOCK_TRUSTED_IPS', '')))), 'lagoon_environment_type' => env('LAGOON_ENVIRONMENT_TYPE', 'development'), 'default_user_group_id_for_unallocated_instances' => env('POLYDOCK_DEFAULT_USER_GROUP_ID_FOR_UNALLOCATED_INSTANCES', 1), 'amazee_ai_backend_private_gpt_settings' => $aisettings, diff --git a/routes/api.php b/routes/api.php index 2ecd0fb5..5201a5ca 100644 --- a/routes/api.php +++ b/routes/api.php @@ -30,12 +30,18 @@ }); }); -Route::post('/register', [RegisterController::class, 'processRegister'])->name('register.process'); -Route::get('/register/{uuid}', [RegisterController::class, 'showRegister'])->name('register.show'); +Route::post('/register', [RegisterController::class, 'processRegister']) + ->name('register.process') + ->middleware('throttle:register'); +Route::get('/register/{uuid}', [RegisterController::class, 'showRegister']) + ->name('register.show') + ->middleware('throttle:public-read'); -Route::get('/regions', [RegionsController::class, 'index'])->name('regions.index'); +Route::get('/regions', [RegionsController::class, 'index']) + ->name('regions.index') + ->middleware('throttle:public-read'); Route::match(['get', 'post'], '/instance/{uuid}/health/{status}', [ PolydockInstanceHealthController::class, '__invoke', -])->name('api.instance.health'); +])->name('api.instance.health')->middleware('throttle:instance-health'); diff --git a/tests/Feature/Api/InstanceHealthApiTest.php b/tests/Feature/Api/InstanceHealthApiTest.php index 3a27b3d7..60e652ee 100644 --- a/tests/Feature/Api/InstanceHealthApiTest.php +++ b/tests/Feature/Api/InstanceHealthApiTest.php @@ -124,4 +124,71 @@ public function test_health_check_does_not_log_plaintext_token_on_error(): void // THEN it should return 400 $response->assertStatus(400); } + + public function test_health_check_from_trusted_ip_bypasses_rate_limit(): void + { + // GIVEN a trusted IP is configured + Config::set('polydock.trusted_ips', ['10.0.0.5']); + Config::set('polydock.health_token', null); + + // WHEN we hit the endpoint 130 times from trusted IP + for ($i = 0; $i < 130; $i++) { + $this->withServerVariables(['REMOTE_ADDR' => '10.0.0.5']) + ->getJson("/api/instance/{$this->uuid}/health/running-healthy-claimed") + ->assertStatus(200); + } + } + + public function test_health_check_with_valid_token_bypasses_rate_limit(): void + { + // GIVEN a token is configured + Config::set('polydock.health_token', 'secure-test-token'); + + // WHEN we hit the endpoint 130 times with the correct token + for ($i = 0; $i < 130; $i++) { + $this->getJson("/api/instance/{$this->uuid}/health/running-healthy-claimed?token=secure-test-token") + ->assertStatus(200); + } + } + + public function test_health_check_throttles_per_ip_across_uuids(): void + { + Config::set('polydock.health_token', null); // No token gating + + // GIVEN another instance exists + $storeApp = PolydockStoreApp::first(); + $group = UserGroup::first(); + + $anotherInstance = new PolydockAppInstance; + $anotherInstance->polydock_store_app_id = $storeApp->id; + $anotherInstance->user_group_id = $group->id; + $anotherInstance->name = 'another-instance'; + $anotherInstance->uuid = (string) Str::uuid(); + $anotherInstance->app_type = PolydockAiApp::class; + $anotherInstance->status = PolydockAppInstanceStatus::RUNNING_HEALTHY_CLAIMED; + $anotherInstance->saveQuietly(); + + // WHEN we exhaust the per-minute limit (120) against the first UUID + for ($i = 0; $i < 120; $i++) { + $this->getJson("/api/instance/{$this->uuid}/health/running-healthy-claimed") + ->assertStatus(200); + } + + // THEN a further request for the first UUID is throttled + $this->getJson("/api/instance/{$this->uuid}/health/running-healthy-claimed") + ->assertStatus(429); + + // AND switching to the second UUID from the same IP is ALSO throttled — + // the limit keys on IP so enumerating UUIDs can't reset the budget + $this->getJson("/api/instance/{$anotherInstance->uuid}/health/running-healthy-claimed") + ->assertStatus(429); + } + + protected function tearDown(): void + { + // Flush cache to reset rate limiters + $this->app['cache']->flush(); + + parent::tearDown(); + } } diff --git a/tests/Feature/Controllers/Api/RegisterControllerTest.php b/tests/Feature/Controllers/Api/RegisterControllerTest.php index d03ad0fc..1ccbaf65 100644 --- a/tests/Feature/Controllers/Api/RegisterControllerTest.php +++ b/tests/Feature/Controllers/Api/RegisterControllerTest.php @@ -14,6 +14,16 @@ class RegisterControllerTest extends TestCase { use RefreshDatabase; + protected function tearDown(): void + { + // Rate-limit counters live in the cache, which RefreshDatabase does not + // reset. Flush it so throttle state can't leak between tests (and make + // the throttle test order-independent). + $this->app['cache']->flush(); + + parent::tearDown(); + } + #[Test] public function it_redacts_sensitive_data_when_processing_registration(): void { @@ -87,4 +97,94 @@ public function it_redacts_sensitive_data_when_showing_registration_status(): vo && $resData['app_admin_password'] === SensitiveDataRedactor::REDACTED_VALUE; })); } + + #[Test] + public function it_throttles_repeated_registration_attempts(): void + { + $payload = [ + 'email' => 'throttle-user@example.com', + 'password' => 'supersecret123', + 'api_key' => 'secret-api-key', + ]; + + // WHEN we post the registration endpoint up to its per-minute limit (10) + for ($i = 0; $i < 10; $i++) { + $this->postJson('/api/register', $payload); + } + + // THEN the 11th request within the same minute is rejected with 429 + $this->postJson('/api/register', $payload) + ->assertStatus(429); + } + + #[Test] + public function it_allows_trusted_ips_to_bypass_registration_throttle(): void + { + // GIVEN a trusted IP is configured + config(['polydock.trusted_ips' => ['10.0.0.5']]); + + $payload = [ + 'email' => 'trusted-throttle-user@example.com', + 'password' => 'supersecret123', + 'api_key' => 'secret-api-key', + ]; + + // WHEN we post 15 times from the trusted IP + for ($i = 0; $i < 15; $i++) { + $this->withServerVariables(['REMOTE_ADDR' => '10.0.0.5']) + ->postJson('/api/register', $payload) + ->assertStatus(202); + } + } + + #[Test] + public function it_throttles_registration_status_requests_per_ip_across_uuids(): void + { + // GIVEN two registrations exist + $registrationA = UserRemoteRegistration::create([ + 'email' => 'user-a@example.com', + 'status' => UserRemoteRegistrationStatusEnum::PENDING, + 'request_data' => [], + ]); + $registrationB = UserRemoteRegistration::create([ + 'email' => 'user-b@example.com', + 'status' => UserRemoteRegistrationStatusEnum::PENDING, + 'request_data' => [], + ]); + + // WHEN we exhaust the per-minute limit (60) against registration A + for ($i = 0; $i < 60; $i++) { + $this->getJson("/api/register/{$registrationA->uuid}") + ->assertStatus(200); + } + + // THEN a further request for registration A is throttled (429) + $this->getJson("/api/register/{$registrationA->uuid}") + ->assertStatus(429); + + // AND switching to registration B from the same IP is ALSO throttled — + // the limit keys on IP so an attacker can't reset it by enumerating UUIDs + $this->getJson("/api/register/{$registrationB->uuid}") + ->assertStatus(429); + } + + #[Test] + public function it_allows_trusted_ips_to_bypass_registration_status_throttle(): void + { + // GIVEN a trusted IP is configured + config(['polydock.trusted_ips' => ['10.0.0.5']]); + + // AND a registration exists + $registration = UserRemoteRegistration::create([ + 'email' => 'user-trusted@example.com', + 'status' => UserRemoteRegistrationStatusEnum::PENDING, + 'request_data' => [], + ]); + + // WHEN we request registration 70 times from the trusted IP + for ($i = 0; $i < 70; $i++) { + $this->getJson("/api/register/{$registration->uuid}", ['REMOTE_ADDR' => '10.0.0.5']) + ->assertStatus(200); + } + } }