From 85e387a39e7dc5add2c72e8a308c4d61dfbc7e24 Mon Sep 17 00:00:00 2001 From: BackNot Date: Mon, 23 Mar 2026 22:39:37 +0200 Subject: [PATCH 01/12] Removed BruteForceService, FailedLogin. Used symfony rate limiter on mfa and login endpoints --- .env | 7 +- composer.json | 2 + composer.lock | 354 +++++++++++++++++- config/packages/framework.yaml | 10 + config/packages/lock.yaml | 2 + src/Entity/Abstraction/FailedLogin.php | 60 --- src/Entity/User.php | 17 - .../LoginRateLimitSubscriber.php | 58 +++ .../MfaRateLimitSubscriber.php | 44 +++ src/EventSubscriber/UserSubscriber.php | 1 - src/Migrations/Version20260323202159.php | 28 ++ .../Abstraction/FailedLoginRepository.php | 38 -- src/Service/BruteForceService.php | 103 ----- symfony.lock | 12 + .../application/auth/rate_limited.html.twig | 20 + translations/application+intl-icu.en.yaml | 2 + 16 files changed, 534 insertions(+), 224 deletions(-) create mode 100644 config/packages/lock.yaml delete mode 100644 src/Entity/Abstraction/FailedLogin.php create mode 100644 src/EventSubscriber/LoginRateLimitSubscriber.php create mode 100644 src/EventSubscriber/MfaRateLimitSubscriber.php create mode 100644 src/Migrations/Version20260323202159.php delete mode 100644 src/Repository/Abstraction/FailedLoginRepository.php delete mode 100644 src/Service/BruteForceService.php create mode 100644 templates/application/auth/rate_limited.html.twig diff --git a/.env b/.env index dbc78cd..e7ee443 100644 --- a/.env +++ b/.env @@ -27,4 +27,9 @@ PHPMAILER_SMTP_PASSWORD= PHPMAILER_SMTP_DEFAULT_SENDER=sammy@codific.com PHPMAILER_SMTP_USE_AUTH=true PHPMAILER_SMTP_DEFAULT_ENCRYPTION=ssl -PHPMAILER_SMTP_AUTO_TLS=true \ No newline at end of file +PHPMAILER_SMTP_AUTO_TLS=true +###> symfony/lock ### +# Choose one of the stores below +# postgresql+advisory://db_user:db_password@localhost/db_name +LOCK_DSN=flock +###< symfony/lock ### diff --git a/composer.json b/composer.json index 6fa4f53..2b00c94 100644 --- a/composer.json +++ b/composer.json @@ -47,6 +47,7 @@ "symfony/html-sanitizer": "*", "symfony/http-client": "*", "symfony/intl": "*", + "symfony/lock": "7.3.*", "symfony/mime": "*", "symfony/monolog-bundle": "*", "symfony/notifier": "*", @@ -54,6 +55,7 @@ "symfony/property-access": "*", "symfony/property-info": "*", "symfony/proxy-manager-bridge": "*", + "symfony/rate-limiter": "7.3.*", "symfony/runtime": "*", "symfony/security-bundle": "*", "symfony/serializer": "*", diff --git a/composer.lock b/composer.lock index a0156f5..d58be81 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "56a9f21606f11ed0f517f7397de1f16b", + "content-hash": "9784f4807d079f45d2599ba7883607a9", "packages": [ { "name": "bacon/bacon-qr-code", @@ -3529,6 +3529,196 @@ }, "time": "2026-02-13T03:05:33+00:00" }, + { + "name": "opis/json-schema", + "version": "2.6.0", + "source": { + "type": "git", + "url": "https://github.com/opis/json-schema.git", + "reference": "8458763e0dd0b6baa310e04f1829fc73da4e8c8a" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/opis/json-schema/zipball/8458763e0dd0b6baa310e04f1829fc73da4e8c8a", + "reference": "8458763e0dd0b6baa310e04f1829fc73da4e8c8a", + "shasum": "" + }, + "require": { + "ext-json": "*", + "opis/string": "^2.1", + "opis/uri": "^1.0", + "php": "^7.4 || ^8.0" + }, + "require-dev": { + "ext-bcmath": "*", + "ext-intl": "*", + "phpunit/phpunit": "^9.0" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "2.x-dev" + } + }, + "autoload": { + "psr-4": { + "Opis\\JsonSchema\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "Apache-2.0" + ], + "authors": [ + { + "name": "Sorin Sarca", + "email": "sarca_sorin@hotmail.com" + }, + { + "name": "Marius Sarca", + "email": "marius.sarca@gmail.com" + } + ], + "description": "Json Schema Validator for PHP", + "homepage": "https://opis.io/json-schema", + "keywords": [ + "json", + "json-schema", + "schema", + "validation", + "validator" + ], + "support": { + "issues": "https://github.com/opis/json-schema/issues", + "source": "https://github.com/opis/json-schema/tree/2.6.0" + }, + "time": "2025-10-17T12:46:48+00:00" + }, + { + "name": "opis/string", + "version": "2.1.0", + "source": { + "type": "git", + "url": "https://github.com/opis/string.git", + "reference": "3e4d2aaff518ac518530b89bb26ed40f4503635e" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/opis/string/zipball/3e4d2aaff518ac518530b89bb26ed40f4503635e", + "reference": "3e4d2aaff518ac518530b89bb26ed40f4503635e", + "shasum": "" + }, + "require": { + "ext-iconv": "*", + "ext-json": "*", + "php": "^7.4 || ^8.0" + }, + "require-dev": { + "phpunit/phpunit": "^9.0" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "2.x-dev" + } + }, + "autoload": { + "psr-4": { + "Opis\\String\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "Apache-2.0" + ], + "authors": [ + { + "name": "Marius Sarca", + "email": "marius.sarca@gmail.com" + }, + { + "name": "Sorin Sarca", + "email": "sarca_sorin@hotmail.com" + } + ], + "description": "Multibyte strings as objects", + "homepage": "https://opis.io/string", + "keywords": [ + "multi-byte", + "opis", + "string", + "string manipulation", + "utf-8" + ], + "support": { + "issues": "https://github.com/opis/string/issues", + "source": "https://github.com/opis/string/tree/2.1.0" + }, + "time": "2025-10-17T12:38:41+00:00" + }, + { + "name": "opis/uri", + "version": "1.1.0", + "source": { + "type": "git", + "url": "https://github.com/opis/uri.git", + "reference": "0f3ca49ab1a5e4a6681c286e0b2cc081b93a7d5a" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/opis/uri/zipball/0f3ca49ab1a5e4a6681c286e0b2cc081b93a7d5a", + "reference": "0f3ca49ab1a5e4a6681c286e0b2cc081b93a7d5a", + "shasum": "" + }, + "require": { + "opis/string": "^2.0", + "php": "^7.4 || ^8.0" + }, + "require-dev": { + "phpunit/phpunit": "^9" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "1.x-dev" + } + }, + "autoload": { + "psr-4": { + "Opis\\Uri\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "Apache-2.0" + ], + "authors": [ + { + "name": "Marius Sarca", + "email": "marius.sarca@gmail.com" + }, + { + "name": "Sorin Sarca", + "email": "sarca_sorin@hotmail.com" + } + ], + "description": "Build, parse and validate URIs and URI-templates", + "homepage": "https://opis.io", + "keywords": [ + "URI Template", + "parse url", + "punycode", + "uri", + "uri components", + "url", + "validate uri" + ], + "support": { + "issues": "https://github.com/opis/uri/issues", + "source": "https://github.com/opis/uri/tree/1.1.0" + }, + "time": "2021-05-22T15:57:08+00:00" + }, { "name": "paragonie/constant_time_encoding", "version": "v3.1.3", @@ -7444,6 +7634,88 @@ ], "time": "2026-01-12T12:03:18+00:00" }, + { + "name": "symfony/lock", + "version": "v7.3.11", + "source": { + "type": "git", + "url": "https://github.com/symfony/lock.git", + "reference": "af564086b6529d1c58d652f5aad8d8851a71f01a" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/symfony/lock/zipball/af564086b6529d1c58d652f5aad8d8851a71f01a", + "reference": "af564086b6529d1c58d652f5aad8d8851a71f01a", + "shasum": "" + }, + "require": { + "php": ">=8.2", + "psr/log": "^1|^2|^3" + }, + "conflict": { + "doctrine/dbal": "<3.6", + "symfony/cache": "<6.4" + }, + "require-dev": { + "doctrine/dbal": "^3.6|^4", + "predis/predis": "^1.1|^2.0" + }, + "type": "library", + "autoload": { + "psr-4": { + "Symfony\\Component\\Lock\\": "" + }, + "exclude-from-classmap": [ + "/Tests/" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Jérémy Derussé", + "email": "jeremy@derusse.com" + }, + { + "name": "Symfony Community", + "homepage": "https://symfony.com/contributors" + } + ], + "description": "Creates and manages locks, a mechanism to provide exclusive access to a shared resource", + "homepage": "https://symfony.com", + "keywords": [ + "cas", + "flock", + "locking", + "mutex", + "redlock", + "semaphore" + ], + "support": { + "source": "https://github.com/symfony/lock/tree/v7.3.11" + }, + "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-01-27T16:12:03+00:00" + }, { "name": "symfony/mime", "version": "v7.3.11", @@ -8901,6 +9173,80 @@ ], "time": "2025-11-02T18:11:54+00:00" }, + { + "name": "symfony/rate-limiter", + "version": "v7.3.11", + "source": { + "type": "git", + "url": "https://github.com/symfony/rate-limiter.git", + "reference": "db2a62a57614a905a41b980aafa92dacae8e1ba3" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/symfony/rate-limiter/zipball/db2a62a57614a905a41b980aafa92dacae8e1ba3", + "reference": "db2a62a57614a905a41b980aafa92dacae8e1ba3", + "shasum": "" + }, + "require": { + "php": ">=8.2", + "symfony/options-resolver": "^7.3" + }, + "require-dev": { + "psr/cache": "^1.0|^2.0|^3.0", + "symfony/lock": "^6.4|^7.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/v7.3.11" + }, + "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-01-27T16:12:03+00:00" + }, { "name": "symfony/routing", "version": "v7.3.10", @@ -15031,7 +15377,7 @@ ], "aliases": [], "minimum-stability": "stable", - "stability-flags": {}, + "stability-flags": [], "prefer-stable": false, "prefer-lowest": false, "platform": { @@ -15046,6 +15392,6 @@ "ext-simplexml": "*", "ext-zip": "*" }, - "platform-dev": {}, - "plugin-api-version": "2.9.0" + "platform-dev": [], + "plugin-api-version": "2.2.0" } diff --git a/config/packages/framework.yaml b/config/packages/framework.yaml index 4b990e6..1978453 100644 --- a/config/packages/framework.yaml +++ b/config/packages/framework.yaml @@ -26,3 +26,13 @@ framework: validation: { enabled: true } + rate_limiter: + login: + policy: sliding_window + limit: 5 + interval: '1 minute' + mfa: + policy: sliding_window + limit: 5 + interval: '1 minute' + diff --git a/config/packages/lock.yaml b/config/packages/lock.yaml new file mode 100644 index 0000000..574879f --- /dev/null +++ b/config/packages/lock.yaml @@ -0,0 +1,2 @@ +framework: + lock: '%env(LOCK_DSN)%' diff --git a/src/Entity/Abstraction/FailedLogin.php b/src/Entity/Abstraction/FailedLogin.php deleted file mode 100644 index 2cbd550..0000000 --- a/src/Entity/Abstraction/FailedLogin.php +++ /dev/null @@ -1,60 +0,0 @@ - - * - * @see http://codific.com - */ - -declare(strict_types=1); - -namespace App\Entity\Abstraction; - -use Doctrine\DBAL\Types\Types; -use Doctrine\ORM\Mapping as ORM; - -#[ORM\Table(name: '`failedlogin`')] -#[ORM\Entity(repositoryClass: "App\Repository\Abstraction\FailedLoginRepository")] -#[ORM\HasLifecycleCallbacks] -class FailedLogin extends AbstractEntity -{ - #[ORM\Column(name: 'username', type: Types::STRING, nullable: true)] - private ?string $username = ''; - - #[ORM\Column(name: 'ip', type: Types::STRING, nullable: true)] - private string $ip = ''; - - public function __toString(): string - { - return ''; - } - - public function setUsername(?string $username): FailedLogin - { - $this->username = $username; - - return $this; - } - - public function getUsername(): ?string - { - return $this->username; - } - - public function setIp(string $ip): FailedLogin - { - $this->ip = $ip; - - return $this; - } - - public function getIp(): string - { - return $this->ip; - } -} diff --git a/src/Entity/User.php b/src/Entity/User.php index 3833083..99c8da7 100644 --- a/src/Entity/User.php +++ b/src/Entity/User.php @@ -89,10 +89,6 @@ class User extends AbstractEntity implements BackupCodeInterface, PasswordResetI #[ORM\Column(name: "`salt`", type: Types::STRING, nullable: true)] protected ?string $salt = ""; - #[Ignore] - #[ORM\Column(name: "`failed_logins`", type: Types::INTEGER)] - protected int $failedLogins = 0; - #[Ignore] #[ORM\Column(name: "`secret_key`", type: Types::STRING, nullable: true)] protected ?string $secretKey = ""; @@ -342,18 +338,6 @@ public function getSalt(): ?string return $this->salt; } - public function setFailedLogins(int $failedLogins): self - { - $this->failedLogins = $failedLogins; - - return $this; - } - - public function getFailedLogins(): int - { - return $this->failedLogins; - } - /** * Erase credentials * @inheritDoc @@ -549,7 +533,6 @@ public function getReadOnlyFields(): array return [ "password", "salt", - "failedLogins", "secretKey", "trustedVersion", "backupCodes", diff --git a/src/EventSubscriber/LoginRateLimitSubscriber.php b/src/EventSubscriber/LoginRateLimitSubscriber.php new file mode 100644 index 0000000..33ec28a --- /dev/null +++ b/src/EventSubscriber/LoginRateLimitSubscriber.php @@ -0,0 +1,58 @@ + ['onKernelRequest', 20], + ]; + } + + public function onKernelRequest(RequestEvent $event): void + { + $request = $event->getRequest(); + + if (!$event->isMainRequest()) { + return; + } + + $route = $request->attributes->get('_route'); + if ($route !== 'app_login_login' || !$request->isMethod('POST')) { + return; + } + + $limiter = $this->loginLimiter->create($request->getClientIp()); + $limit = $limiter->consume(1); + + if (!$limit->isAccepted()) { + $retryAfter = $limit->getRetryAfter()->getTimestamp() - time(); + $event->setResponse(new Response( + $this->twig->render('application/auth/rate_limited.html.twig', [ + 'retry_after' => $retryAfter, + ]), + Response::HTTP_TOO_MANY_REQUESTS, + ['Retry-After' => $retryAfter] + )); + } + } +} diff --git a/src/EventSubscriber/MfaRateLimitSubscriber.php b/src/EventSubscriber/MfaRateLimitSubscriber.php new file mode 100644 index 0000000..6a651df --- /dev/null +++ b/src/EventSubscriber/MfaRateLimitSubscriber.php @@ -0,0 +1,44 @@ + ['onMfaAttempt', 20], + TwoFactorAuthenticationEvents::FAILURE => 'onMfaFailure', + ]; + } + + public function onMfaAttempt(TwoFactorAuthenticationEvent $event): void + { + $request = $event->getRequest(); + $limiter = $this->mfaLimiter->create($request->getClientIp()); + if ($limiter->consume(0)->isAccepted() === false) { + throw new TooManyLoginAttemptsAuthenticationException(); + } + } + + public function onMfaFailure(TwoFactorAuthenticationEvent $event): void + { + $request = $event->getRequest(); + $limiter = $this->mfaLimiter->create($request->getClientIp()); + $limiter->consume(1); + } +} diff --git a/src/EventSubscriber/UserSubscriber.php b/src/EventSubscriber/UserSubscriber.php index 6d10d29..7ae47ee 100644 --- a/src/EventSubscriber/UserSubscriber.php +++ b/src/EventSubscriber/UserSubscriber.php @@ -103,7 +103,6 @@ public function onUserAuthenticated(UserAuthenticatedEvent $event) $user->setLastLogin(new \DateTime('now')); $user->setPasswordResetHash(null); $user->setPasswordResetHashExpiration(null); - $user->setFailedLogins(0); $this->entityManager->flush(); $request = $event->getRequest(); diff --git a/src/Migrations/Version20260323202159.php b/src/Migrations/Version20260323202159.php new file mode 100644 index 0000000..cdecf32 --- /dev/null +++ b/src/Migrations/Version20260323202159.php @@ -0,0 +1,28 @@ +addSql('DROP TABLE IF EXISTS `failedlogin`'); + $this->addSql('ALTER TABLE `user` DROP COLUMN `failed_logins`'); + } + + public function down(Schema $schema): void + { + $this->addSql('CREATE TABLE `failedlogin` (id INT AUTO_INCREMENT NOT NULL, username VARCHAR(255) DEFAULT NULL, ip VARCHAR(255) DEFAULT NULL, created_at DATETIME DEFAULT CURRENT_TIMESTAMP, updated_at DATETIME DEFAULT CURRENT_TIMESTAMP, deleted_at DATETIME DEFAULT NULL, PRIMARY KEY(id)) DEFAULT CHARACTER SET utf8mb4 COLLATE `utf8mb4_unicode_ci` ENGINE = InnoDB'); + $this->addSql('ALTER TABLE `user` ADD `failed_logins` INT NOT NULL DEFAULT 0'); + } +} diff --git a/src/Repository/Abstraction/FailedLoginRepository.php b/src/Repository/Abstraction/FailedLoginRepository.php deleted file mode 100644 index 69ae001..0000000 --- a/src/Repository/Abstraction/FailedLoginRepository.php +++ /dev/null @@ -1,38 +0,0 @@ - - * - * @see http://codific.com - */ - -declare(strict_types=1); - -namespace App\Repository\Abstraction; - -use App\Entity\Abstraction\FailedLogin; -use Doctrine\Persistence\ManagerRegistry; - -/** - * Class FailedLoginRepository. - * - * @method FailedLogin|null find($id, $lockMode = null, $lockVersion = null) - * @method FailedLogin|null findOneBy(array $criteria, array $orderBy = null) - * @method FailedLogin[] findAll() - * @method FailedLogin[] findBy(array $criteria, array $orderBy = null, $limit = null, $offset = null) - */ -class FailedLoginRepository extends AbstractRepository -{ - /** - * FailedLoginRepository constructor. - */ - public function __construct(ManagerRegistry $registry, string $entityClassName = FailedLogin::class) - { - parent::__construct($registry, $entityClassName); - } -} diff --git a/src/Service/BruteForceService.php b/src/Service/BruteForceService.php deleted file mode 100644 index 1b9772b..0000000 --- a/src/Service/BruteForceService.php +++ /dev/null @@ -1,103 +0,0 @@ - - * - * @see http://codific.com - */ - -declare(strict_types=1); - -namespace App\Service; - -use App\Entity\Abstraction\FailedLogin; -use Doctrine\ORM\EntityManagerInterface; -use Doctrine\ORM\NonUniqueResultException; -use Symfony\Component\HttpFoundation\Request; - -class BruteForceService -{ - private Request $request; - - /** - * Use when retrieving the number of recent failed logins. - * In minutes. - */ - private int $timeFrame = 10; - - /** - * Threshold values - * Example: for 15 failed logins user will be not allowed to login for next 60 seconds. - */ - private array $threshold = [ - 15 => 60, - 25 => 120, - 35 => 240, - ]; - - public function __construct( - private readonly EntityManagerInterface $entityManager - ) { - $this->request = Request::createFromGlobals(); - } - - public function addFailAttempt(string $username = ''): void - { - $failedLogin = new FailedLogin(); - $failedLogin->setUsername($username); - $failedLogin->setIp($this->request->getClientIp()); - $this->entityManager->persist($failedLogin); - $this->entityManager->flush(); - } - - /** - * @param string $filter use IP(Default) or USERNAME or BOTH to choice how to check for login delay - * @param string $username if you choice USERNAME or BOTH for filter you must supply user name here - * - * @throws NonUniqueResultException - */ - public function getDelay(string $filter = 'IP', string $username = ''): int - { - $repoFailedLogin = $this->entityManager->getRepository(FailedLogin::class); - $queryBuilder = $repoFailedLogin->createQueryBuilder('a'); - $queryBuilder = $queryBuilder->select('count(a.id) as count, max(a.createdAt) as lastDate') - ->where('a.createdAt > :timeframe') - ->setParameter('timeframe', new \DateTime("-{$this->timeFrame} minutes")); - - if (strtoupper($filter) === 'IP') { - $queryBuilder->andWhere('a.ip = :ip') - ->setParameter('ip', $this->request->getClientIp()); - } - if (strtoupper($filter) === 'USERNAME') { - $queryBuilder->andWhere('a.username = :username') - ->setParameter('username', $username); - } - if (strtoupper($filter) === 'BOTH') { - $queryBuilder->andWhere('a.username = :username AND a.ip = :ip') - ->setParameter('username', $username) - ->setParameter('ip', $this->request->getClientIp()); - } - - $result = $queryBuilder->getQuery()->getOneOrNullResult(); - if ($result === null) { - return 0; - } - - $failedAttempts = $result['count']; - $lastFailedTimestamp = $result['lastDate']; - - krsort($this->threshold); - foreach ($this->threshold as $attempts => $delay) { - if ($failedAttempts > $attempts && time() < (strtotime($lastFailedTimestamp) + $delay)) { - return (strtotime($lastFailedTimestamp) + $delay) - time(); - } - } - - return 0; - } -} diff --git a/symfony.lock b/symfony.lock index 626cd50..5a389ff 100644 --- a/symfony.lock +++ b/symfony.lock @@ -198,6 +198,18 @@ "src/Kernel.php" ] }, + "symfony/lock": { + "version": "7.3", + "recipe": { + "repo": "github.com/symfony/recipes", + "branch": "main", + "version": "5.2", + "ref": "8e937ff2b4735d110af1770f242c1107fdab4c8e" + }, + "files": [ + "config/packages/lock.yaml" + ] + }, "symfony/maker-bundle": { "version": "1.50", "recipe": { diff --git a/templates/application/auth/rate_limited.html.twig b/templates/application/auth/rate_limited.html.twig new file mode 100644 index 0000000..58b9fd8 --- /dev/null +++ b/templates/application/auth/rate_limited.html.twig @@ -0,0 +1,20 @@ +{% extends 'application/base_login.html.twig' %} + +{% block title %}{{ 'application.general.login_title'|trans({}, 'application') }}{% endblock %} + +{% block body %} +
+
+
+
+
+ {{ 'application.general.too_many_login_attempts'|trans({'%seconds%': retry_after}, 'application') }} +
+
+ {{ 'application.general.back_to_login'|trans({}, 'application') }} +
+
+
+
+
+{% endblock %} diff --git a/translations/application+intl-icu.en.yaml b/translations/application+intl-icu.en.yaml index 9380a23..2ad66dc 100644 --- a/translations/application+intl-icu.en.yaml +++ b/translations/application+intl-icu.en.yaml @@ -103,6 +103,8 @@ application : login_title : Welcome to SAMMY login_credentials : Please enter your login credentials login_invalid_credentials : Your login credentials could not be validated + too_many_login_attempts : Too many login attempts. Please try again in {seconds, plural, one {# second} other {# seconds}}. + back_to_login : Back to login forgot_password : I forgot my password signin_with_gitlab : Sign in with your GitLab account signin_with_github : Sign in with your GitHub account From 584cddef97041e024eb9475e26877c63de9438a0 Mon Sep 17 00:00:00 2001 From: BackNot Date: Fri, 17 Apr 2026 16:51:51 +0300 Subject: [PATCH 02/12] Replaced Content disposition header. Replaced characters in filename --- src/Controller/Application/ReportingController.php | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/Controller/Application/ReportingController.php b/src/Controller/Application/ReportingController.php index f0c412d..b9a51db 100644 --- a/src/Controller/Application/ReportingController.php +++ b/src/Controller/Application/ReportingController.php @@ -14,6 +14,7 @@ use App\Service\ScoreService; use Doctrine\ORM\NonUniqueResultException; use Symfony\Component\HttpFoundation\BinaryFileResponse; +use Symfony\Component\HttpFoundation\HeaderUtils; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Routing\Annotation\Route; @@ -136,12 +137,19 @@ public function exportSammJson(SammExportService $sammExportService, ProjectServ $frameworkName = $project->getMetamodel()?->getName() ?? 'SAMM'; $filename = sprintf('%s_%s_%s.samm.json', $project->getName(), $frameworkName, uniqid()); + $asciiFilename = preg_replace('/[^A-Za-z0-9._-]/', '_', $filename); - return new Response($json, Response::HTTP_OK, [ + $response = new Response($json, Response::HTTP_OK, [ 'Content-Type' => 'application/json', - 'Content-Disposition' => sprintf('attachment; filename="%s"', $filename), 'Content-Length' => strlen($json), ]); + $response->headers->set('Content-Disposition', HeaderUtils::makeDisposition( + HeaderUtils::DISPOSITION_ATTACHMENT, + $filename, + $asciiFilename + )); + + return $response; } #[Route('/overviewPartial/{id}', name: 'overviewPartial', requirements: ['id' => "\d+"], methods: ['GET'])] From aa810e63aec3eac2ac4ba44bd4c073538ff68737 Mon Sep 17 00:00:00 2001 From: BackNot Date: Fri, 17 Apr 2026 21:37:23 +0300 Subject: [PATCH 03/12] Added project access check on export --- src/Controller/Application/ReportingController.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Controller/Application/ReportingController.php b/src/Controller/Application/ReportingController.php index b9a51db..fe78e81 100644 --- a/src/Controller/Application/ReportingController.php +++ b/src/Controller/Application/ReportingController.php @@ -110,6 +110,7 @@ public function exportSamm(AssessmentExporterService $assessmentExporterService, return $this->safeRedirect($request, 'app_index'); } $project = $projectService->getCurrentProject(); + $this->denyAccessUnlessGranted('PROJECT_ACCESS', $project); $assessment = $project->getAssessment(); $filePath = $assessmentExporterService->getToolbox($assessment, $project); @@ -125,6 +126,7 @@ public function exportSammJson(SammExportService $sammExportService, ProjectServ } $project = $projectService->getCurrentProject(); + $this->denyAccessUnlessGranted('PROJECT_ACCESS', $project); $assessment = $project->getAssessment(); try { From 1d42715da22675063c1a48d3705abea620196571 Mon Sep 17 00:00:00 2001 From: BackNot Date: Fri, 17 Apr 2026 22:19:05 +0300 Subject: [PATCH 04/12] Hide password reset hash. Mails are sent immediately --- config/packages/framework.yaml | 4 + .../Application/LoginController.php | 9 ++- src/Controller/Application/UserController.php | 19 +++++ .../Abstraction/PasswordResetInterface.php | 11 +++ src/Entity/User.php | 16 ++++ .../PasswordResetRateLimitSubscriber.php | 56 ++++++++++++++ src/Repository/UserRepository.php | 5 +- src/Service/MailingService.php | 77 +++++++++++++++++-- src/Service/ResetPasswordService.php | 13 +++- src/Service/UserService.php | 4 +- .../user/partials/_user_row.html.twig | 26 +++++++ translations/application+intl-icu.en.yaml | 6 ++ translations/application+intl-icu.es.yaml | 6 ++ 13 files changed, 234 insertions(+), 18 deletions(-) create mode 100644 src/EventSubscriber/PasswordResetRateLimitSubscriber.php diff --git a/config/packages/framework.yaml b/config/packages/framework.yaml index 1978453..368496c 100644 --- a/config/packages/framework.yaml +++ b/config/packages/framework.yaml @@ -35,4 +35,8 @@ framework: policy: sliding_window limit: 5 interval: '1 minute' + password_reset: + policy: sliding_window + limit: 5 + interval: '1 hour' diff --git a/src/Controller/Application/LoginController.php b/src/Controller/Application/LoginController.php index 57d1812..6232542 100644 --- a/src/Controller/Application/LoginController.php +++ b/src/Controller/Application/LoginController.php @@ -133,14 +133,19 @@ public function resetPasswordRequest( if ($form->isSubmitted() && $form->isValid()) { $email = $form->get('email')->getData(); $user = $userRepository->findOneBy(['email' => $email, 'deletedAt' => null]); + $sendFailed = false; if ($user !== null && !in_array(Role::ADMINISTRATOR->string(), $user->getRoles(), true)) { $status = $passwordResetService->reset($user); if ($status) { - $mailingService->add(\App\Enum\MailTemplateType::USER_PASSWORD_RESET, $user); + $sendFailed = !$mailingService->sendImmediate(\App\Enum\MailTemplateType::USER_PASSWORD_RESET, $user); } } - $this->addFlash('success', $translator->trans('application.general.reset_password_success', [], 'application')); + if ($sendFailed) { + $this->addFlash('error', $translator->trans('application.general.reset_password_send_failed', [], 'application')); + } else { + $this->addFlash('success', $translator->trans('application.general.reset_password_success', [], 'application')); + } return $this->redirectToRoute('app_login_login'); } diff --git a/src/Controller/Application/UserController.php b/src/Controller/Application/UserController.php index 7cc3e29..c602c49 100644 --- a/src/Controller/Application/UserController.php +++ b/src/Controller/Application/UserController.php @@ -7,6 +7,7 @@ use App\DTO\NewUserDTO; use App\Entity\Group; use App\Entity\User; +use App\Enum\MailTemplateType; use App\Enum\Role; use App\Event\Admin\Create\UserCreatedEvent; use App\Exception\BadGroupForUserSuppliedException; @@ -178,6 +179,24 @@ public function delete(Request $request, User $chosenUser, UserService $userServ return $this->safeRedirect($request, 'app_user_index'); } + #[Route('/resend-welcome/{id}', name: 'resend_welcome', requirements: ['id' => "\d+"], methods: ['POST'])] + #[IsGranted('USER_EDIT', 'chosenUser')] + public function resendWelcome(Request $request, User $chosenUser, UserService $userService): Response + { + if (!$this->isCsrfTokenValid((string) $this->getUser()?->getId(), $request->request->get('_token'))) { + return $this->safeRedirect($request, 'app_user_index'); + } + + $sent = $userService->welcomeUser($chosenUser, MailTemplateType::USER_WELCOME, flush: true); + if ($sent) { + $this->addFlash('success', $this->translator->trans('application.user.resend_welcome_success', ['user' => trim("$chosenUser")], 'application')); + } else { + $this->addFlash('error', $this->translator->trans('application.user.resend_welcome_error', [], 'application')); + } + + return $this->safeRedirect($request, 'app_user_index'); + } + #[Route('/editUser/{id}', name: 'edituser', requirements: ['id' => "\d+"], methods: ['POST'])] #[IsGranted('USER_EDIT', 'chosenUser')] public function editUser(Request $request, User $chosenUser, UserService $userService): RedirectResponse diff --git a/src/Entity/Abstraction/PasswordResetInterface.php b/src/Entity/Abstraction/PasswordResetInterface.php index 81d58c3..912913d 100644 --- a/src/Entity/Abstraction/PasswordResetInterface.php +++ b/src/Entity/Abstraction/PasswordResetInterface.php @@ -30,4 +30,15 @@ public function getPasswordResetHash(): ?string; public function setPasswordResetHashExpiration(?\DateTime $passwordResetHashExpiration); public function getPasswordResetHashExpiration(): ?\DateTime; + + /** + * Transient, in-memory only. Holds the plaintext token during the request + * in which the reset was generated so the mailer can embed it in the link. + * The persisted column stores a SHA-256 hash of this value. + * + * @return mixed + */ + public function setPlaintextPasswordResetHash(?string $plaintext); + + public function getPlaintextPasswordResetHash(): ?string; } diff --git a/src/Entity/User.php b/src/Entity/User.php index 99c8da7..a8c3677 100644 --- a/src/Entity/User.php +++ b/src/Entity/User.php @@ -112,6 +112,9 @@ class User extends AbstractEntity implements BackupCodeInterface, PasswordResetI #[ORM\Column(name: "`password_reset_hash_expiration`", type: Types::DATETIME_MUTABLE, nullable: true)] protected ?\DateTime $passwordResetHashExpiration = null; + #[Ignore] + protected ?string $plaintextPasswordResetHash = null; + #[ORM\OneToMany(mappedBy: "assignedTo", targetEntity: Stage::class, cascade: ["persist"], fetch: "LAZY", orphanRemoval: false)] #[ORM\OrderBy(["id" => "ASC"])] #[MaxDepth(1)] @@ -444,6 +447,19 @@ public function getPasswordResetHashExpiration(): ?\DateTime return $this->passwordResetHashExpiration; } + public function setPlaintextPasswordResetHash(?string $plaintext): self + { + $this->plaintextPasswordResetHash = $plaintext; + + return $this; + } + + #[Ignore] + public function getPlaintextPasswordResetHash(): ?string + { + return $this->plaintextPasswordResetHash; + } + /** * This method is a copy constructor that will return a copy object (except for the id field) * Note that this method will not save the object diff --git a/src/EventSubscriber/PasswordResetRateLimitSubscriber.php b/src/EventSubscriber/PasswordResetRateLimitSubscriber.php new file mode 100644 index 0000000..457dfc0 --- /dev/null +++ b/src/EventSubscriber/PasswordResetRateLimitSubscriber.php @@ -0,0 +1,56 @@ + ['onKernelRequest', 20], + ]; + } + + public function onKernelRequest(RequestEvent $event): void + { + $request = $event->getRequest(); + + if (!$event->isMainRequest()) { + return; + } + + $route = $request->attributes->get('_route'); + if ($route !== 'app_login_reset_password_request' || !$request->isMethod('POST')) { + return; + } + + $limiter = $this->passwordResetLimiter->create($request->getClientIp()); + $limit = $limiter->consume(1); + + if (!$limit->isAccepted()) { + $retryAfter = $limit->getRetryAfter()->getTimestamp() - time(); + $event->setResponse(new Response( + $this->twig->render('application/auth/rate_limited.html.twig', [ + 'retry_after' => $retryAfter, + ]), + Response::HTTP_TOO_MANY_REQUESTS, + ['Retry-After' => $retryAfter] + )); + } + } +} diff --git a/src/Repository/UserRepository.php b/src/Repository/UserRepository.php index 63ec65c..fcb6b8e 100644 --- a/src/Repository/UserRepository.php +++ b/src/Repository/UserRepository.php @@ -21,6 +21,7 @@ use App\Interface\EntityInterface; use App\Pagination\Paginator; use App\Repository\Abstraction\AbstractRepository; +use App\Service\ResetPasswordService; use Doctrine\ORM\NonUniqueResultException; use Doctrine\ORM\Query\Expr\Join; use Doctrine\ORM\Query\ResultSetMapping; @@ -199,7 +200,7 @@ public function loadUserByPasswordResetHash(string $hash): ?User ->andWhere('user.deletedAt IS NULL') ->andWhere('user.passwordResetHashExpiration > CURRENT_TIMESTAMP()') ->setParameter('adminRole', '"'.Role::ADMINISTRATOR->string().'"') - ->setParameter('hash', $hash); + ->setParameter('hash', ResetPasswordService::hashToken($hash)); $user = $qb->getQuery()->getOneOrNullResult(); if ($user === null) { @@ -221,7 +222,7 @@ public function loadAdminByPasswordResetHash(string $hash): ?User ->andWhere('user.deletedAt IS NULL') ->andWhere('user.passwordResetHashExpiration > CURRENT_TIMESTAMP()') ->setParameter('adminRole', '"'.Role::ADMINISTRATOR->string().'"') - ->setParameter('hash', $hash); + ->setParameter('hash', ResetPasswordService::hashToken($hash)); $user = $qb->getQuery()->getOneOrNullResult(); if ($user === null) { diff --git a/src/Service/MailingService.php b/src/Service/MailingService.php index 1aa272d..f6c9635 100644 --- a/src/Service/MailingService.php +++ b/src/Service/MailingService.php @@ -37,6 +37,12 @@ public function __construct( } public function add(MailTemplateType $mailTemplateType, User $user, array $extraPlaceholders = [], string $attachment = ''): void + { + $mailTemplateEntity = $this->resolveTemplate($mailTemplateType); + $this->addCustom($user, $mailTemplateEntity, $extraPlaceholders, $attachment); + } + + private function resolveTemplate(MailTemplateType $mailTemplateType): ?MailTemplate { $mailTemplateEntity = $this->mailTemplateRepository->findOneBy(['type' => $mailTemplateType]); if ($mailTemplateEntity === null) { @@ -56,26 +62,80 @@ public function add(MailTemplateType $mailTemplateType, User $user, array $extra $this->entityManager->flush(); } } - $this->addCustom($user, $mailTemplateEntity, $extraPlaceholders, $attachment); + + return $mailTemplateEntity; + } + + /** + * Send an email synchronously without queueing its rendered body. + * Used for one-shot messages that carry sensitive links (password reset, + * welcome). On success, a Mailing row with '[redacted]' as the body is + * persisted for audit; the rendered link never touches the DB. + */ + public function sendImmediate(MailTemplateType $mailTemplateType, User $user, array $extraPlaceholders = [], string $attachment = ''): bool + { + $template = $this->resolveTemplate($mailTemplateType); + if ($template === null) { + return false; + } + + [$subject, $message, $resolvedAttachment] = $this->replacePlaceholders( + $template->getSubject(), + $template->getMessage(), + $user, + $extraPlaceholders, + $attachment + ); + + $result = $this->sendMail( + $user->getEmail(), + "{$user->getName()} {$user->getSurname()}", + $subject, + $message, + $resolvedAttachment !== '' ? $resolvedAttachment : null + ); + + if ($result === false) { + $this->logger->log(LogLevel::ERROR, 'Immediate mail send failed', [ + 'templateType' => $mailTemplateType->value, + 'userId' => $user->getId(), + ]); + + return false; + } + + $mailing = $this->buildMailing($user, $template, $subject, '[redacted]', $resolvedAttachment); + $mailing->setStatus(\App\Enum\MailingStatus::SENT); + $mailing->setSentDate(new \DateTime()); + $this->entityManager->persist($mailing); + $this->entityManager->flush(); + + return true; } public function addCustom(User $user, ?MailTemplate $template, array $extraPlaceholders = [], string $attachment = ''): void + { + [$subject, $message, $attachment] = $this->replacePlaceholders($template->getSubject(), $template->getMessage(), $user, $extraPlaceholders, $attachment); + $mailing = $this->buildMailing($user, $template, $subject, $message, $attachment); + $this->entityManager->persist($mailing); + $this->entityManager->flush(); + } + + private function buildMailing(User $user, ?MailTemplate $template, string $subject, string $message, ?string $attachment): Mailing { $mailing = new Mailing(); $mailing->setEmail($user->getEmail()); - $isUserAdmin = in_array(Role::ADMINISTRATOR->string(), $user->getRoles(), true); - if ($isUserAdmin) { + if (in_array(Role::ADMINISTRATOR->string(), $user->getRoles(), true)) { $mailing->setUser($user); } $mailing->setName($user->getName()); $mailing->setSurname($user->getSurname()); - [$subject, $message, $attachment] = $this->replacePlaceholders($template->getSubject(), $template->getMessage(), $user, $extraPlaceholders, $attachment); $mailing->setSubject($subject); $mailing->setMessage($message); $mailing->setMailTemplate($template); $mailing->setAttachment($attachment); - $this->entityManager->persist($mailing); - $this->entityManager->flush(); + + return $mailing; } private function replacePlaceholders(string $subject, string $message, User $user, array $extraPlaceholders = [], ?string $attachment = null): array @@ -90,8 +150,9 @@ private function replacePlaceholders(string $subject, string $message, User $use $subject = str_ireplace($find, $replace, $subject); $message = str_ireplace($find, $replace, $message); - if (stristr($message, '[link]') !== false && $user->getPasswordResetHash() !== null && (strlen($user->getPasswordResetHash()) > 0)) { - $link = $this->urlGenerator->generate($urlName, ['hash' => $user->getPasswordResetHash()], $this->urlGenerator::ABSOLUTE_URL); + $plaintextResetToken = $user->getPlaintextPasswordResetHash(); + if (stristr($message, '[link]') !== false && $plaintextResetToken !== null && $plaintextResetToken !== '') { + $link = $this->urlGenerator->generate($urlName, ['hash' => $plaintextResetToken], $this->urlGenerator::ABSOLUTE_URL); $message = str_ireplace('[link]', $link, $message); $message = str_ireplace('[linkValidity]', $user->getPasswordResetHashExpiration()->format('d-M-Y @ H:i'), $message); } diff --git a/src/Service/ResetPasswordService.php b/src/Service/ResetPasswordService.php index 61f11ec..f9a817a 100644 --- a/src/Service/ResetPasswordService.php +++ b/src/Service/ResetPasswordService.php @@ -11,7 +11,7 @@ class ResetPasswordService { - private string $passwordResetHashValid = '+8 hours'; + private string $passwordResetHashValid = '+1 hour'; private string $welcomeResetHashValid = '+8 hours'; public function __construct( @@ -24,6 +24,11 @@ public function __construct( $this->welcomeResetHashValid = (string) $this->configurationService->get('welcomeResetHashValid', $this->welcomeResetHashValid); } + public static function hashToken(string $plaintext): string + { + return hash('sha256', $plaintext); + } + public function reset(PasswordResetInterface $entity, bool $isWelcomeMail = false): bool { $resetHashValid = $this->passwordResetHashValid; @@ -32,9 +37,9 @@ public function reset(PasswordResetInterface $entity, bool $isWelcomeMail = fals } try { - if ($entity->getPasswordResetHash() === '' || (new \DateTime()) > $entity->getPasswordResetHashExpiration()) { - $entity->setPasswordResetHash($this->generator->base32(16)); - } + $plaintext = $this->generator->base32(16); + $entity->setPlaintextPasswordResetHash($plaintext); + $entity->setPasswordResetHash(self::hashToken($plaintext)); $entity->setPasswordResetHashExpiration(new \DateTime($resetHashValid)); $this->entityManager->flush(); diff --git a/src/Service/UserService.php b/src/Service/UserService.php index 74eb2db..3bf91a0 100644 --- a/src/Service/UserService.php +++ b/src/Service/UserService.php @@ -159,12 +159,12 @@ public function getUsersWithProjectAccess(Project $project, $neededRole = null): public function welcomeUser(User $user, MailTemplateType $mailTemplate, bool $flush = false): bool { $this->resetPasswordService->reset($user, true); - $this->mailingService->add($mailTemplate, $user); + $sent = $this->mailingService->sendImmediate($mailTemplate, $user); if ($flush) { $this->entityManager->flush(); } - return true; + return $sent; } diff --git a/templates/application/user/partials/_user_row.html.twig b/templates/application/user/partials/_user_row.html.twig index 81e8ddb..af9861b 100644 --- a/templates/application/user/partials/_user_row.html.twig +++ b/templates/application/user/partials/_user_row.html.twig @@ -45,6 +45,11 @@ + + + {% if user.id is not same as app.user.id %} {% endif %} +