From 47a984a4137329413ce053ea3af2c35e02508aa2 Mon Sep 17 00:00:00 2001 From: vgreb Date: Tue, 9 Jun 2026 22:33:23 +0200 Subject: [PATCH] Remplacement de ActionThrottling par symfony/rate-limiter --- app/config/packages/rate_limiter.yaml | 6 + app/config/packages/ting.yaml | 7 - app/config/reference.php | 2 +- composer.json | 1 + composer.lock | 76 +++++++++- phpstan-baseline.php | 60 -------- .../Command/CleanThrottlingCommand.php | 31 ---- .../Event/Ticket/SponsorTicketAction.php | 18 +-- .../ActionThrottling/ActionThrottling.php | 46 ------ .../Security/ActionThrottling/Log.php | 122 ---------------- .../ActionThrottling/LogRepository.php | 138 ------------------ 11 files changed, 90 insertions(+), 417 deletions(-) create mode 100644 app/config/packages/rate_limiter.yaml delete mode 100644 sources/AppBundle/Command/CleanThrottlingCommand.php delete mode 100644 sources/AppBundle/Security/ActionThrottling/ActionThrottling.php delete mode 100644 sources/AppBundle/Security/ActionThrottling/Log.php delete mode 100644 sources/AppBundle/Security/ActionThrottling/LogRepository.php diff --git a/app/config/packages/rate_limiter.yaml b/app/config/packages/rate_limiter.yaml new file mode 100644 index 000000000..52885137d --- /dev/null +++ b/app/config/packages/rate_limiter.yaml @@ -0,0 +1,6 @@ +framework: + rate_limiter: + sponsor_token: + policy: sliding_window + limit: 10 + interval: '1 hour' diff --git a/app/config/packages/ting.yaml b/app/config/packages/ting.yaml index 94df47d7b..e076991d3 100644 --- a/app/config/packages/ting.yaml +++ b/app/config/packages/ting.yaml @@ -38,10 +38,3 @@ ting: default: connection: main database: "%database_name%" - throttling: - namespace : AppBundle\Security\ActionThrottling - directory : "@AppBundle/Security/ActionThrottling" - options: - default: - connection: main - database: '%database_name%' diff --git a/app/config/reference.php b/app/config/reference.php index 7193b06b3..1ddd0e95b 100644 --- a/app/config/reference.php +++ b/app/config/reference.php @@ -629,7 +629,7 @@ * }>, * }, * rate_limiter?: bool|array{ // Rate limiter configuration - * enabled?: bool|Param, // Default: false + * enabled?: bool|Param, // Default: true * limiters?: array=8.4.1", + "symfony/options-resolver": "^7.4|^8.0" + }, + "require-dev": { + "psr/cache": "^1.0|^2.0|^3.0", + "symfony/lock": "^7.4|^8.0" + }, + "type": "library", + "autoload": { + "psr-4": { + "Symfony\\Component\\RateLimiter\\": "" + }, + "exclude-from-classmap": [ + "/Tests/" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Wouter de Jong", + "email": "wouter@wouterj.nl" + }, + { + "name": "Symfony Community", + "homepage": "https://symfony.com/contributors" + } + ], + "description": "Provides a Token Bucket implementation to rate limit input and output in your application", + "homepage": "https://symfony.com", + "keywords": [ + "limiter", + "rate-limiter" + ], + "support": { + "source": "https://github.com/symfony/rate-limiter/tree/v8.1.0" + }, + "funding": [ + { + "url": "https://symfony.com/sponsor", + "type": "custom" + }, + { + "url": "https://github.com/fabpot", + "type": "github" + }, + { + "url": "https://github.com/nicolas-grekas", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", + "type": "tidelift" + } + ], + "time": "2026-05-29T05:06:50+00:00" + }, { "name": "symfony/routing", "version": "v8.1.0", diff --git a/phpstan-baseline.php b/phpstan-baseline.php index 0a1e27671..b7cd05c2e 100644 --- a/phpstan-baseline.php +++ b/phpstan-baseline.php @@ -11431,66 +11431,6 @@ 'count' => 1, 'path' => __DIR__ . '/sources/AppBundle/Routing/LegacyRouter.php', ]; -$ignoreErrors[] = [ - 'message' => '#^Method AppBundle\\\\Security\\\\ActionThrottling\\\\ActionThrottling\\:\\:clearLogsForIp\\(\\) has parameter \\$action with no type specified\\.$#', - 'identifier' => 'missingType.parameter', - 'count' => 1, - 'path' => __DIR__ . '/sources/AppBundle/Security/ActionThrottling/ActionThrottling.php', -]; -$ignoreErrors[] = [ - 'message' => '#^Method AppBundle\\\\Security\\\\ActionThrottling\\\\ActionThrottling\\:\\:clearLogsForIp\\(\\) has parameter \\$ip with no type specified\\.$#', - 'identifier' => 'missingType.parameter', - 'count' => 1, - 'path' => __DIR__ . '/sources/AppBundle/Security/ActionThrottling/ActionThrottling.php', -]; -$ignoreErrors[] = [ - 'message' => '#^Parameter \\#1 \\$action of method AppBundle\\\\Security\\\\ActionThrottling\\\\LogRepository\\:\\:removeLogs\\(\\) expects string, mixed given\\.$#', - 'identifier' => 'argument.type', - 'count' => 1, - 'path' => __DIR__ . '/sources/AppBundle/Security/ActionThrottling/ActionThrottling.php', -]; -$ignoreErrors[] = [ - 'message' => '#^Parameter \\#1 \\$ip of method AppBundle\\\\Security\\\\ActionThrottling\\\\Log\\:\\:setIp\\(\\) expects string, string\\|null given\\.$#', - 'identifier' => 'argument.type', - 'count' => 1, - 'path' => __DIR__ . '/sources/AppBundle/Security/ActionThrottling/ActionThrottling.php', -]; -$ignoreErrors[] = [ - 'message' => '#^Parameter \\#1 \\$objectId of method AppBundle\\\\Security\\\\ActionThrottling\\\\Log\\:\\:setObjectId\\(\\) expects int, int\\|null given\\.$#', - 'identifier' => 'argument.type', - 'count' => 1, - 'path' => __DIR__ . '/sources/AppBundle/Security/ActionThrottling/ActionThrottling.php', -]; -$ignoreErrors[] = [ - 'message' => '#^Parameter \\#2 \\$ip of method AppBundle\\\\Security\\\\ActionThrottling\\\\LogRepository\\:\\:removeLogs\\(\\) expects string, mixed given\\.$#', - 'identifier' => 'argument.type', - 'count' => 1, - 'path' => __DIR__ . '/sources/AppBundle/Security/ActionThrottling/ActionThrottling.php', -]; -$ignoreErrors[] = [ - 'message' => '#^Method AppBundle\\\\Security\\\\ActionThrottling\\\\LogRepository\\:\\:getApplicableLogs\\(\\) should return array\\{ip\\: int, object\\: int\\} but returns mixed\\.$#', - 'identifier' => 'return.type', - 'count' => 1, - 'path' => __DIR__ . '/sources/AppBundle/Security/ActionThrottling/LogRepository.php', -]; -$ignoreErrors[] = [ - 'message' => '#^Method AppBundle\\\\Security\\\\ActionThrottling\\\\LogRepository\\:\\:initMetadata\\(\\) has parameter \\$options with no value type specified in iterable type array\\.$#', - 'identifier' => 'missingType.iterableValue', - 'count' => 1, - 'path' => __DIR__ . '/sources/AppBundle/Security/ActionThrottling/LogRepository.php', -]; -$ignoreErrors[] = [ - 'message' => '#^Method AppBundle\\\\Security\\\\ActionThrottling\\\\LogRepository\\:\\:initMetadata\\(\\) should return M of CCMBenchmark\\\\Ting\\\\Repository\\\\Metadata but returns CCMBenchmark\\\\Ting\\\\Repository\\\\Metadata\\\\.$#', - 'identifier' => 'return.type', - 'count' => 1, - 'path' => __DIR__ . '/sources/AppBundle/Security/ActionThrottling/LogRepository.php', -]; -$ignoreErrors[] = [ - 'message' => '#^Parameter \\#1 \\$databaseName of method CCMBenchmark\\\\Ting\\\\Repository\\\\Metadata\\\\:\\:setDatabase\\(\\) expects string, mixed given\\.$#', - 'identifier' => 'argument.type', - 'count' => 1, - 'path' => __DIR__ . '/sources/AppBundle/Security/ActionThrottling/LogRepository.php', -]; $ignoreErrors[] = [ 'message' => '#^Method AppBundle\\\\Security\\\\GithubAuthenticator\\:\\:onAuthenticationSuccess\\(\\) has parameter \\$firewallName with no type specified\\.$#', 'identifier' => 'missingType.parameter', diff --git a/sources/AppBundle/Command/CleanThrottlingCommand.php b/sources/AppBundle/Command/CleanThrottlingCommand.php deleted file mode 100644 index f57e844a2..000000000 --- a/sources/AppBundle/Command/CleanThrottlingCommand.php +++ /dev/null @@ -1,31 +0,0 @@ -setName('throttling:clean'); - } - - protected function execute(InputInterface $input, OutputInterface $output): int - { - $this->actionThrottling->clearOldLogs(); - - return Command::SUCCESS; - } -} diff --git a/sources/AppBundle/Controller/Event/Ticket/SponsorTicketAction.php b/sources/AppBundle/Controller/Event/Ticket/SponsorTicketAction.php index cbc84a40a..51ba5fc9f 100644 --- a/sources/AppBundle/Controller/Event/Ticket/SponsorTicketAction.php +++ b/sources/AppBundle/Controller/Event/Ticket/SponsorTicketAction.php @@ -6,16 +6,16 @@ use AppBundle\Controller\Event\EventActionHelper; use AppBundle\Event\Model\Repository\SponsorTicketRepository; -use AppBundle\Security\ActionThrottling\ActionThrottling; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpFoundation\Session\Session; +use Symfony\Component\RateLimiter\RateLimiterFactoryInterface; final class SponsorTicketAction extends AbstractController { public function __construct( - private readonly ActionThrottling $actionThrottling, + private readonly RateLimiterFactoryInterface $sponsorTokenLimiter, private readonly EventActionHelper $eventActionHelper, private readonly SponsorTicketRepository $sponsorTicketRepository, ) {} @@ -39,19 +39,15 @@ public function __invoke(Request $request, $eventSlug): Response $errors[] = 'Token absent'; } else { $token = $request->request->get('sponsor_token'); + $limiter = $this->sponsorTokenLimiter->create($request->getClientIp()); + $rateLimit = $limiter->consume(1); $sponsorTicket = $this->sponsorTicketRepository->getOneBy(['token' => $token]); - if ( - $this->actionThrottling->isActionBlocked('sponsor_token', $request->getClientIp()) - || $sponsorTicket === null - ) { - // Si l'IP a fait trop de tentatives, on affiche le meme message que si le token n'existe pas - // L'ip est bloquée pendant un temps mais il ne faut pas en informer celui qui tente - pour éviter - // qu'il ne change d'IP + if (!$rateLimit->isAccepted() || $sponsorTicket === null) { + // Même message que si le token n'existe pas, pour ne pas révéler le blocage $errors[] = 'Ce token n\'existe pas.'; - $this->actionThrottling->log('sponsor_token', $request->getClientIp()); } else { + $limiter->reset(); $session->set('sponsor_ticket_id', $sponsorTicket->getId()); - $this->actionThrottling->clearLogsForIp('sponsor_token', $request->getClientIp()); return $this->redirectToRoute('sponsor_ticket_form', ['eventSlug' => $eventSlug]); } diff --git a/sources/AppBundle/Security/ActionThrottling/ActionThrottling.php b/sources/AppBundle/Security/ActionThrottling/ActionThrottling.php deleted file mode 100644 index 4283af266..000000000 --- a/sources/AppBundle/Security/ActionThrottling/ActionThrottling.php +++ /dev/null @@ -1,46 +0,0 @@ -logRepository->getApplicableLogs($ip, $objectId, new \DateInterval($delay)); - - return $logs['ip'] > $limitations[$action]['limit'] || $logs['object'] > $limitations[$action]['limit']; - } - - public function clearLogsForIp($action, $ip): void - { - $this->logRepository->removeLogs($action, $ip); - } - - public function log(string $action, ?string $ip = null, ?int $objectId = null): void - { - $log = new Log(); - $log - ->setCreatedOn(new \DateTime()) - ->setAction($action) - ->setIp($ip) - ->setObjectId($objectId) - ; - $this->logRepository->save($log); - } - - public function clearOldLogs(): void - { - $this->logRepository->clearOldLogs(new \DateInterval('P30D')); - } -} diff --git a/sources/AppBundle/Security/ActionThrottling/Log.php b/sources/AppBundle/Security/ActionThrottling/Log.php deleted file mode 100644 index ed58844d8..000000000 --- a/sources/AppBundle/Security/ActionThrottling/Log.php +++ /dev/null @@ -1,122 +0,0 @@ - ['delay' => 'PT1H', 'limit' => 10], - ]; - - /** - * @var int - */ - private $id; - - /** - * @var string - */ - private $ip; - - /** - * @var string - */ - private $action; - - /** - * @var int - */ - private $objectId; - - private ?\DateTime $createdOn = null; - - /** - * @return int - */ - public function getId() - { - return $this->id; - } - - /** - * @param int $id - */ - public function setId($id): self - { - $this->id = $id; - return $this; - } - - /** - * @return string - */ - public function getIp() - { - return $this->ip; - } - - /** - * @param string $ip - */ - public function setIp($ip): self - { - $this->ip = $ip; - return $this; - } - - /** - * @return string - */ - public function getAction() - { - return $this->action; - } - - /** - * @param string $action - */ - public function setAction($action): self - { - $this->action = $action; - return $this; - } - - /** - * @return int - */ - public function getObjectId() - { - return $this->objectId; - } - - /** - * @param int $objectId - */ - public function setObjectId($objectId): self - { - $this->objectId = $objectId; - return $this; - } - - /** - * @return \DateTime - */ - public function getCreatedOn(): ?\DateTime - { - return $this->createdOn; - } - - public function setCreatedOn(\DateTime $createdOn): self - { - $this->createdOn = $createdOn; - return $this; - } -} diff --git a/sources/AppBundle/Security/ActionThrottling/LogRepository.php b/sources/AppBundle/Security/ActionThrottling/LogRepository.php deleted file mode 100644 index bc7bd3721..000000000 --- a/sources/AppBundle/Security/ActionThrottling/LogRepository.php +++ /dev/null @@ -1,138 +0,0 @@ - - */ -class LogRepository extends Repository implements MetadataInitializer -{ - /** - * @return array{ip: int, object: int} - */ - public function getApplicableLogs(?string $ip, ?int $objectId, \DateInterval $interval): array - { - if ($ip === null && $objectId === null) { - throw new \RuntimeException('I need at least an ip or an object Id to get logs'); - } - - /** - * @var Select $query - */ - $query = $this->getQueryBuilder(self::QUERY_SELECT); - $query - ->cols(['COUNT(ip) AS ip', 'COUNT(object_id) AS object']) - ->from('afup_throttling') - ->where('created_on > :createdOn') - ; - $createdOn = new \DateTime(); - $createdOn->sub($interval); - - $params = [ - 'createdOn' => $createdOn->format('Y-m-d H:i:s'), - ]; - - $where = []; - if ($ip !== null) { - $where[] = 'ip = :ip'; - $params['ip'] = ip2long($ip); - } - if ($objectId !== null) { - $where[] = 'object_id = :id'; - $params['id'] = $objectId; - } - $query->where(' (' . implode(' OR ', $where) . ' )'); - - return $this - ->getPreparedQuery($query->getStatement()) - ->setParams($params) - ->query($this->getCollection(new HydratorArray()))->first() - ; - } - - /** - * @param string $action - * @param string $ip - */ - public function removeLogs($action, $ip): void - { - $query = $this->getPreparedQuery(' - DELETE FROM afup_throttling - WHERE `action` = :action - AND `ip` = :ip - '); - $query->setParams([ - 'action' => $action, - 'ip' => ip2long($ip), - ]); - - $query->execute(); - } - - public function clearOldLogs(\DateInterval $delay): void - { - $query = $this->getPreparedQuery(' - DELETE FROM afup_throttling - WHERE `created_on` < :date - '); - $date = new \DateTime()->sub($delay); - $query->setParams([ - 'date' => $date->format('Y-m-d H:i:s'), - ]); - - $query->execute(); - } - - /** - * @inheritDoc - */ - public static function initMetadata(SerializerFactoryInterface $serializerFactory, array $options = []) - { - $metadata = new Metadata($serializerFactory); - $metadata->setEntity(Log::class); - $metadata->setConnectionName('main'); - $metadata->setDatabase($options['database']); - $metadata->setTable('afup_throttling'); - - $metadata - ->addField([ - 'columnName' => 'id', - 'fieldName' => 'id', - 'primary' => true, - 'autoincrement' => true, - 'type' => 'int', - ]) - ->addField([ - 'columnName' => 'ip', - 'fieldName' => 'ip', - 'type' => 'ip', - ]) - ->addField([ - 'columnName' => 'action', - 'fieldName' => 'action', - 'type' => 'string', - ]) - ->addField([ - 'columnName' => 'object_id', - 'fieldName' => 'objectId', - 'type' => 'int', - ]) - ->addField([ - 'columnName' => 'created_on', - 'fieldName' => 'createdOn', - 'type' => 'datetime', - ]) - ; - - return $metadata; - } -}