diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index 971b928..76f9520 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -54,6 +54,21 @@ class AppServiceProvider extends ServiceProvider RateLimiter::for('api', fn (Request $request) => Limit::perMinute(120) ->by($request->user()?->id ?: $request->ip())); + // Backs POST /api/v1/auth/token. Budgeted per account and address + // rather than per address alone, because the deployed MCP server + // (mcp/) signs everyone in from one container: per IP, one person's + // wrong password — or a stranger's guesses — would lock every other + // user out of it. The per-address ceiling still bounds how many + // accounts a single client can try. + RateLimiter::for('auth-token', fn (Request $request) => [ + Limit::perMinute(6)->by('auth-token:'.sha1( + Str::lower((string) ($request->input('username') ?: $request->input('refresh_token'))) + .'|'.$request->ip() + )), + + Limit::perMinute(30)->by('auth-token-ip:'.$request->ip()), + ]); + // The public API is a different problem. The public frontend renders // server-side, so every visitor's page view arrives from the same IP: // a per-IP budget sized for one person would throttle the entire diff --git a/public/openapi.yaml b/public/openapi.yaml index 0bbb129..d1c0acc 100644 --- a/public/openapi.yaml +++ b/public/openapi.yaml @@ -909,8 +909,9 @@ paths: description: | Proxies the identity server's `password` and `refresh_token` grants so first-party clients need not embed the OAuth server's URL or secret. - Rate limited to 6 requests per minute. Browser clients should use - Authorization Code + PKCE instead. + Rate limited to 6 requests per minute per account and address, and 30 + per address overall. Browser clients should use Authorization Code + + PKCE instead. security: [] requestBody: required: true diff --git a/routes/api.php b/routes/api.php index b78cb70..de9e49d 100644 --- a/routes/api.php +++ b/routes/api.php @@ -152,7 +152,7 @@ Route::prefix('v1')->name('api.v1.')->group(function () { // Convenience endpoint for first-party clients: exchanges credentials for // a token upstream so they do not need to know the identity server's URL. Route::post('auth/token', [TokenController::class, 'store']) - ->middleware('throttle:6,1') + ->middleware('throttle:auth-token') ->name('auth.token'); Route::middleware(['auth:api', 'throttle:api'])->group(function () { diff --git a/tests/Feature/Api/TokenEndpointTest.php b/tests/Feature/Api/TokenEndpointTest.php index 8856774..35f1da3 100644 --- a/tests/Feature/Api/TokenEndpointTest.php +++ b/tests/Feature/Api/TokenEndpointTest.php @@ -103,6 +103,38 @@ class TokenEndpointTest extends TestCase ->assertJsonValidationErrors('grant_type'); } + #[Test] + public function one_account_exhausting_its_budget_does_not_lock_out_another_on_the_same_address(): void + { + // Everyone the MCP server signs in arrives from its one container. + // Registered first so it wins over fakePireon()'s token stub, which + // would otherwise accept every password. + Http::fake(["{$this->pireonServer}/oauth/token" => fn ($request) => $request['username'] === 'ada@example.com' + ? Http::response(['access_token' => 'fake-access-token', 'token_type' => 'Bearer', 'expires_in' => 3600]) + : Http::response(['error' => 'invalid_grant'], 400)]); + $this->fakePireon(); + + foreach (range(1, 6) as $attempt) { + $this->postJson('/api/v1/auth/token', [ + 'grant_type' => 'password', + 'username' => 'mallory@example.com', + 'password' => "guess-{$attempt}", + ])->assertStatus(422); + } + + $this->postJson('/api/v1/auth/token', [ + 'grant_type' => 'password', + 'username' => 'MALLORY@example.com', + 'password' => 'guess-7', + ])->assertTooManyRequests(); + + $this->postJson('/api/v1/auth/token', [ + 'grant_type' => 'password', + 'username' => 'ada@example.com', + 'password' => 'secret', + ])->assertOk(); + } + #[Test] public function repeated_calls_with_the_same_token_hit_the_identity_server_once(): void {