diff --git a/app/Http/RequestHandlers/ContactAction.php b/app/Http/RequestHandlers/ContactAction.php index 19bc4d91be0..2364751031f 100644 --- a/app/Http/RequestHandlers/ContactAction.php +++ b/app/Http/RequestHandlers/ContactAction.php @@ -28,6 +28,7 @@ use Fisharebest\Webtrees\Services\CaptchaService; use Fisharebest\Webtrees\Services\EmailService; use Fisharebest\Webtrees\Services\MessageService; +use Fisharebest\Webtrees\Services\RateLimitService; use Fisharebest\Webtrees\Services\UserService; use Fisharebest\Webtrees\Tree; use Fisharebest\Webtrees\Validator; @@ -56,26 +57,31 @@ class ContactAction implements RequestHandlerInterface private MessageService $message_service; + private RateLimitService $rate_limit_service; + private UserService $user_service; /** * MessagePage constructor. * - * @param CaptchaService $captcha_service - * @param EmailService $email_service - * @param MessageService $message_service - * @param UserService $user_service + * @param CaptchaService $captcha_service + * @param EmailService $email_service + * @param MessageService $message_service + * @param RateLimitService $rate_limit_service + * @param UserService $user_service */ public function __construct( CaptchaService $captcha_service, EmailService $email_service, MessageService $message_service, + RateLimitService $rate_limit_service, UserService $user_service ) { - $this->captcha_service = $captcha_service; - $this->email_service = $email_service; - $this->user_service = $user_service; - $this->message_service = $message_service; + $this->captcha_service = $captcha_service; + $this->email_service = $email_service; + $this->user_service = $user_service; + $this->rate_limit_service = $rate_limit_service; + $this->message_service = $message_service; } /** @@ -138,6 +144,8 @@ public function handle(ServerRequestInterface $request): ResponseInterface $sender = new GuestUser($from_email, $from_name); + $this->rate_limit_service->limitRateForUser($to_user, 20, 1200, 'rate-limit-contact'); + if ($this->message_service->deliverMessage($sender, $to_user, $subject, $body, $url, $ip)) { FlashMessages::addMessage(I18N::translate('The message was successfully sent to %s.', e($to_user->realName())), 'success'); diff --git a/app/Http/RequestHandlers/PasswordRequestAction.php b/app/Http/RequestHandlers/PasswordRequestAction.php index 7ade888b829..290edcf72ca 100644 --- a/app/Http/RequestHandlers/PasswordRequestAction.php +++ b/app/Http/RequestHandlers/PasswordRequestAction.php @@ -25,6 +25,7 @@ use Fisharebest\Webtrees\I18N; use Fisharebest\Webtrees\Log; use Fisharebest\Webtrees\Services\EmailService; +use Fisharebest\Webtrees\Services\RateLimitService; use Fisharebest\Webtrees\Services\UserService; use Fisharebest\Webtrees\SiteUser; use Fisharebest\Webtrees\Tree; @@ -49,18 +50,25 @@ class PasswordRequestAction implements RequestHandlerInterface, StatusCodeInterf private EmailService $email_service; + private RateLimitService $rate_limit_service; + private UserService $user_service; /** * PasswordRequestForm constructor. * - * @param EmailService $email_service - * @param UserService $user_service + * @param EmailService $email_service + * @param RateLimitService $rate_limit_service + * @param UserService $user_service */ - public function __construct(EmailService $email_service, UserService $user_service) - { - $this->user_service = $user_service; - $this->email_service = $email_service; + public function __construct( + EmailService $email_service, + RateLimitService $rate_limit_service, + UserService $user_service + ) { + $this->email_service = $email_service; + $this->rate_limit_service = $rate_limit_service; + $this->user_service = $user_service; } /** @@ -78,6 +86,8 @@ public function handle(ServerRequestInterface $request): ResponseInterface $user = $this->user_service->findByEmail($email); if ($user instanceof User) { + $this->rate_limit_service->limitRateForUser($user, 5, 300, 'rate-limit-pw-reset'); + $token = Str::random(self::TOKEN_LENGTH); $expire = (string) Carbon::now()->addHour()->getTimestamp(); $url = route(PasswordResetPage::class, [ diff --git a/app/Http/RequestHandlers/RegisterAction.php b/app/Http/RequestHandlers/RegisterAction.php index 48121ff007e..8da9f515913 100644 --- a/app/Http/RequestHandlers/RegisterAction.php +++ b/app/Http/RequestHandlers/RegisterAction.php @@ -29,6 +29,7 @@ use Fisharebest\Webtrees\NoReplyUser; use Fisharebest\Webtrees\Services\CaptchaService; use Fisharebest\Webtrees\Services\EmailService; +use Fisharebest\Webtrees\Services\RateLimitService; use Fisharebest\Webtrees\Services\UserService; use Fisharebest\Webtrees\Session; use Fisharebest\Webtrees\Site; @@ -54,23 +55,28 @@ class RegisterAction implements RequestHandlerInterface private EmailService $email_service; + private RateLimitService $rate_limit_service; + private UserService $user_service; /** * RegisterController constructor. * - * @param CaptchaService $captcha_service - * @param EmailService $email_service - * @param UserService $user_service + * @param CaptchaService $captcha_service + * @param EmailService $email_service + * @param RateLimitService $rate_limit_service + * @param UserService $user_service */ public function __construct( CaptchaService $captcha_service, EmailService $email_service, + RateLimitService $rate_limit_service, UserService $user_service ) { - $this->captcha_service = $captcha_service; - $this->email_service = $email_service; - $this->user_service = $user_service; + $this->captcha_service = $captcha_service; + $this->email_service = $email_service; + $this->rate_limit_service = $rate_limit_service; + $this->user_service = $user_service; } /** @@ -116,6 +122,8 @@ public function handle(ServerRequestInterface $request): ResponseInterface return redirect(route(RegisterPage::class)); } + $this->rate_limit_service->limitRateForSite(5, 300, 'rate-limit-registration'); + Log::addAuthenticationLog('User registration requested for: ' . $username); $user = $this->user_service->create($username, $realname, $email, $password); diff --git a/app/Services/RateLimitService.php b/app/Services/RateLimitService.php new file mode 100644 index 00000000000..de49d62aecd --- /dev/null +++ b/app/Services/RateLimitService.php @@ -0,0 +1,124 @@ +. + */ + +declare(strict_types=1); + +namespace Fisharebest\Webtrees\Services; + +use Fisharebest\Webtrees\Contracts\UserInterface; +use Fisharebest\Webtrees\Http\Exceptions\HttpTooManyRequestsException; +use Fisharebest\Webtrees\Site; +use LogicException; + +use function array_filter; +use function explode; +use function intdiv; +use function strlen; +use function time; + +/** + * Throttle events to prevent abuse. + */ +class RateLimitService +{ + private int $now; + + /** + * + */ + public function __construct() + { + $this->now = time(); + } + + /** + * Rate limit for actions related to a user, such as password reset request. + * Allow $num requests every $seconds + * + * @param int $num allow this number of events + * @param int $seconds in a rolling window of this number of seconds + * @param string $limit name of limit to enforce + * + * @return void + */ + public function limitRateForSite(int $num, int $seconds, string $limit): void + { + $history = Site::getPreference($limit); + + $history = $this->checkLimitReached($num, $seconds, $history); + + Site::setPreference($limit, $history); + } + + /** + * Rate limit for actions related to a user, such as password reset request. + * Allow $num requests every $seconds + * + * @param UserInterface $user limit events for this user + * @param int $num allow this number of events + * @param int $seconds in a rolling window of this number of seconds + * @param string $limit name of limit to enforce + * + * @return void + */ + public function limitRateForUser(UserInterface $user, int $num, int $seconds, string $limit): void + { + $history = $user->getPreference($limit); + + $history = $this->checkLimitReached($num, $seconds, $history); + + $user->setPreference($limit, $history); + } + + /** + * Rate limit - allow $num requests every $seconds + * + * @param int $num allow this number of events + * @param int $seconds in a rolling window of this number of seconds + * @param string $history comma-separated list of previous timestamps + * + * @return string updated list of timestamps + * @throws HttpTooManyRequestsException + */ + private function checkLimitReached(int $num, int $seconds, string $history): string + { + // Make sure we can store enough previous timestamps in a database field. + $max = intdiv(256, strlen($this->now . ',')); + if ($num > $max) { + throw new LogicException('Cannot store ' . $num . ' previous events in the database'); + } + + // Extract the timestamps. + $timestamps = array_filter(explode(',', $history)); + + // Filter events within our time window. + $filter = fn(string $x): bool => (int) $x >= $this->now - $seconds && (int) $x <= $this->now; + $in_window = array_filter($timestamps, $filter); + + if (count($in_window) >= $num) { + throw new HttpTooManyRequestsException(); + } + + $timestamps[] = (string) $this->now; + + while (count($timestamps) > $max) { + array_shift($timestamps); + } + + return implode(',', $timestamps); + } +} diff --git a/tests/app/Services/RateLimitServiceTest.php b/tests/app/Services/RateLimitServiceTest.php new file mode 100644 index 00000000000..d2af39e204e --- /dev/null +++ b/tests/app/Services/RateLimitServiceTest.php @@ -0,0 +1,113 @@ +. + */ + +declare(strict_types=1); + +namespace Fisharebest\Webtrees\Services; + +use Fisharebest\Webtrees\GuestUser; +use Fisharebest\Webtrees\Http\Exceptions\HttpTooManyRequestsException; +use Fisharebest\Webtrees\TestCase; +use LogicException; + +use function explode; +use function implode; +use function range; +use function time; + +/** + * Test harness for the class RateLimitService + */ +class RateLimitServiceTest extends TestCase +{ + /** + * @covers \Fisharebest\Webtrees\Services\RateLimitService + * + * @return void + */ + public function testTooMuchHistory(): void + { + $rate_limit_service = new RateLimitService(); + + $user = new GuestUser(); + + $this->expectException(LogicException::class); + + $rate_limit_service->limitRateForUser($user, 1000, 30, 'rate-limit'); + } + + /** + * @covers \Fisharebest\Webtrees\Services\RateLimitService + * + * @return void + */ + public function testLimitNotReached(): void + { + $rate_limit_service = new RateLimitService(); + + $user = new GuestUser(); + + $rate_limit_service->limitRateForUser($user, 3, 30, 'rate-limit'); + $history = $user->getPreference('rate-limit'); + $this->assertCount(1, explode(',', $history)); + + $rate_limit_service->limitRateForUser($user, 3, 30, 'rate-limit'); + $history = $user->getPreference('rate-limit'); + $this->assertCount(2, explode(',', $history)); + + $rate_limit_service->limitRateForUser($user, 3, 30, 'rate-limit'); + $history = $user->getPreference('rate-limit'); + $this->assertCount(3, explode(',', $history)); + } + + /** + * @covers \Fisharebest\Webtrees\Services\RateLimitService + * + * @return void + */ + public function testOldEventsIgnored(): void + { + $rate_limit_service = new RateLimitService(); + + $user = new GuestUser(); + + $history = implode(',', range(time() - 35, time() - 31)); + $user->setPreference('rate-limit', $history); + + $rate_limit_service->limitRateForUser($user, 5, 30, 'rate-limit'); + $history = $user->getPreference('rate-limit'); + $this->assertCount(6, explode(',', $history)); + } + + /** + * @covers \Fisharebest\Webtrees\Services\RateLimitService + * + * @return void + */ + public function testLimitReached(): void + { + $rate_limit_service = new RateLimitService(); + + $user = new GuestUser(); + + $history = implode(',', range(time() - 5, time() - 1)); + $user->setPreference('rate-limit', $history); + + $this->expectException(HttpTooManyRequestsException::class); + $rate_limit_service->limitRateForUser($user, 5, 30, 'rate-limit'); + } +}