diff --git a/app/Actions/Fortify/CreateNewUser.php b/app/Actions/Fortify/CreateNewUser.php index 44a03c17d..ea249ad4f 100644 --- a/app/Actions/Fortify/CreateNewUser.php +++ b/app/Actions/Fortify/CreateNewUser.php @@ -4,7 +4,6 @@ use App\Models\Team; use App\Models\User; -use Illuminate\Http\Request; use Illuminate\Support\Facades\Hash; use Illuminate\Support\Facades\RateLimiter; use Illuminate\Support\Facades\Validator; @@ -22,8 +21,6 @@ class CreateNewUser implements CreatesNewUsers private const REGISTRATION_EMAIL_IDENTITY_DECAY_SECONDS = 3600; - public function __construct(private readonly Request $request) {} - /** * Validate and create a newly registered user. * @@ -95,7 +92,7 @@ private function ensureRegistrationIsNotRateLimited(array $input): void { $keys = [ [ - 'key' => 'registration:ip:'.sha1($this->realIp()), + 'key' => 'registration:ip:'.sha1((string) request()->ip()), 'max' => self::REGISTRATION_IP_MAX_ATTEMPTS, 'decay' => self::REGISTRATION_IP_DECAY_SECONDS, ], @@ -120,9 +117,4 @@ private function ensureRegistrationIsNotRateLimited(array $input): void RateLimiter::hit($limit['key'], $limit['decay']); } } - - private function realIp(): string - { - return $this->request->server('REMOTE_ADDR') ?? $this->request->ip(); - } } diff --git a/app/Providers/FortifyServiceProvider.php b/app/Providers/FortifyServiceProvider.php index 65d968774..ce16e617d 100644 --- a/app/Providers/FortifyServiceProvider.php +++ b/app/Providers/FortifyServiceProvider.php @@ -9,10 +9,8 @@ use App\Models\OauthSetting; use App\Models\TeamInvitation; use App\Models\User; -use Illuminate\Cache\RateLimiting\Limit; use Illuminate\Http\Request; use Illuminate\Support\Facades\Hash; -use Illuminate\Support\Facades\RateLimiter; use Illuminate\Support\ServiceProvider; use Laravel\Fortify\Contracts\RegisterResponse; use Laravel\Fortify\Fortify; @@ -122,45 +120,5 @@ public function boot(): void Fortify::twoFactorChallengeView(function () { return view('auth.two-factor-challenge'); }); - - RateLimiter::for('force-password-reset', function (Request $request) { - return Limit::perMinute(15)->by($request->user()->id); - }); - - RateLimiter::for('forgot-password', function (Request $request) { - // Use real client IP (not spoofable forwarded headers) - $realIp = $request->server('REMOTE_ADDR') ?? $request->ip(); - - $limits = [ - Limit::perMinutes(10, 3)->by('forgot-password:ip:'.sha1($realIp)), - ]; - - $emailIdentity = normalize_email_identity($request->input('email')); - if ($emailIdentity !== null) { - $limits[] = Limit::perHour(3)->by('forgot-password:email-identity:'.sha1($emailIdentity)); - } - - return $limits; - }); - - RateLimiter::for('login', function (Request $request) { - $email = (string) $request->email; - // Use email + real client IP (not spoofable forwarded headers) - // server('REMOTE_ADDR') gives the actual connecting IP before proxy headers - $realIp = $request->server('REMOTE_ADDR') ?? $request->ip(); - - return Limit::perMinute(5)->by($email.'|'.$realIp); - }); - - RateLimiter::for('magic-link', function (Request $request) { - $realIp = $request->server('REMOTE_ADDR') ?? $request->ip(); - $token = (string) $request->input('token'); - - return Limit::perMinute(5)->by(hash('sha256', $token.'|'.$realIp)); - }); - - RateLimiter::for('two-factor', function (Request $request) { - return Limit::perMinute(5)->by($request->session()->get('login.id')); - }); } } diff --git a/app/Providers/RouteServiceProvider.php b/app/Providers/RouteServiceProvider.php index 4068572c8..e07d39a08 100644 --- a/app/Providers/RouteServiceProvider.php +++ b/app/Providers/RouteServiceProvider.php @@ -58,5 +58,34 @@ protected function configureRateLimiting(): void RateLimiter::for('feedback', function (Request $request) { return Limit::perMinute(3)->by($request->user()?->id ?: $request->ip()); }); + + RateLimiter::for('login', function (Request $request) { + return Limit::perMinute(5)->by((string) $request->email.'|'.$request->ip()); + }); + + RateLimiter::for('two-factor', function (Request $request) { + return Limit::perMinute(5)->by($request->session()->get('login.id')); + }); + + RateLimiter::for('forgot-password', function (Request $request) { + $limits = [ + Limit::perMinutes(10, 3)->by('forgot-password:ip:'.sha1((string) $request->ip())), + ]; + + $emailIdentity = normalize_email_identity($request->input('email')); + if ($emailIdentity !== null) { + $limits[] = Limit::perHour(3)->by('forgot-password:email-identity:'.sha1($emailIdentity)); + } + + return $limits; + }); + + RateLimiter::for('magic-link', function (Request $request) { + return Limit::perMinute(5)->by(hash('sha256', (string) $request->input('token').'|'.$request->ip())); + }); + + RateLimiter::for('force-password-reset', function (Request $request) { + return Limit::perMinute(15)->by($request->user()->id); + }); } } diff --git a/tests/Feature/Auth/LoginRateLimitIPTest.php b/tests/Feature/Auth/LoginRateLimitIPTest.php index e1b85ae5c..04e6925f4 100644 --- a/tests/Feature/Auth/LoginRateLimitIPTest.php +++ b/tests/Feature/Auth/LoginRateLimitIPTest.php @@ -8,7 +8,7 @@ uses(RefreshDatabase::class); beforeEach(function () { - InstanceSettings::updateOrCreate(['id' => 0]); + InstanceSettings::forceCreate(['id' => 0]); RateLimiter::clear('login'); $this->user = User::factory()->create([ diff --git a/tests/Feature/RegistrationRateLimitTest.php b/tests/Feature/RegistrationRateLimitTest.php index 7130ed36e..5007c41d8 100644 --- a/tests/Feature/RegistrationRateLimitTest.php +++ b/tests/Feature/RegistrationRateLimitTest.php @@ -72,6 +72,43 @@ ->assertTooManyRequests(); }); +it('keeps clients behind the same reverse proxy in separate ip rate limit buckets', function () { + foreach (range(1, 3) as $attempt) { + $this->withServerVariables(['REMOTE_ADDR' => '172.18.0.2']) + ->withHeaders(['X-Forwarded-For' => '203.0.113.50']) + ->post('/register', [ + 'name' => "Proxied User {$attempt}", + 'email' => "proxied{$attempt}@example.com", + 'password' => 'Password1!@', + 'password_confirmation' => 'Password1!@', + ]) + ->assertRedirect(); + + auth()->logout(); + $this->flushSession(); + } + + $this->withServerVariables(['REMOTE_ADDR' => '172.18.0.2']) + ->withHeaders(['X-Forwarded-For' => '203.0.113.50']) + ->post('/register', [ + 'name' => 'Blocked User', + 'email' => 'blocked-proxied@example.com', + 'password' => 'Password1!@', + 'password_confirmation' => 'Password1!@', + ]) + ->assertTooManyRequests(); + + $this->withServerVariables(['REMOTE_ADDR' => '172.18.0.2']) + ->withHeaders(['X-Forwarded-For' => '203.0.113.51']) + ->post('/register', [ + 'name' => 'Other Client', + 'email' => 'other-proxied@example.com', + 'password' => 'Password1!@', + 'password_confirmation' => 'Password1!@', + ]) + ->assertRedirect(); +}); + it('keeps distinct dotted and plus-addressed mailboxes in separate rate limit buckets on ordinary domains', function () { $registrationIpKey = 'registration:ip:'.sha1('127.0.0.1'); $emails = [