diff --git a/.github/avancement.md b/.github/avancement.md index 553b3696..bee7aab9 100644 --- a/.github/avancement.md +++ b/.github/avancement.md @@ -1,6 +1,6 @@ # 📋 Avancement — HomeCloud API -> Dernière mise à jour : 2026-03-16 +> Dernière mise à jour : 2026-03-24 > **Status git :** `main` — tout mergé, 301 tests ✅ @@ -43,6 +43,29 @@ src/ --- +## ✅ Audit Sécurité — TERMINÉ (2026-03-24) + +Score global : **8/10** — aucun point critique détecté. + +| Point audité | Résultat | +|---|---| +| Upload validation (MIME, extension, path traversal) | ✅ Excellent | +| JWT RS256 + Refresh Token rotation | ✅ OK | +| Autorisation (OwnershipChecker + Voter) | ✅ OK | +| Mots de passe (argon2id/bcrypt) | ✅ OK | +| CORS (regex serrée en prod) | ✅ OK | +| Security Headers (CSP, X-Frame-Options…) | ✅ OK | +| SQL injection (QueryBuilder paramétrisé) | ✅ OK | +| Rate limiting sur /api/v1/auth/login | ❌ À faire | +| Auth failure logging | ⚠️ À améliorer | +| HSTS header | ⚠️ À faire | +| `composer audit` en CI | ⚠️ À faire | +| Assert sur DTOs | ⚠️ Basse priorité | + +→ Plan de remédiation : `.github/todo-security.md` + +--- + ## ⚠️ Bugs connus | Priorité | Bug | Détail | diff --git a/.github/todo-security.md b/.github/todo-security.md new file mode 100644 index 00000000..d56e1644 --- /dev/null +++ b/.github/todo-security.md @@ -0,0 +1,82 @@ +# Todo : Sécurité — Remédiation post-audit + +> Audit réalisé le 2026-03-24 — Score initial : 8/10 +> Aucun point critique. Ce plan couvre les 4 axes d'amélioration identifiés. + +--- + +## 🔴 Priorité HAUTE + +### 1. Rate Limiting sur `/api/v1/auth/login` + +**Risque :** Brute force sur l'endpoint de login — aucun mécanisme de blocage en place. + +- [ ] `composer require symfony/rate-limiter` +- [ ] Configurer un limiter dans `config/packages/rate_limiter.yaml` (ex : 5 tentatives / 15 min par IP) +- [ ] Activer dans `config/packages/security.yaml` → firewall `login` → `login_throttling` +- [ ] Écrire un test fonctionnel : `testLoginIsThrottledAfterFiveFailures` (RED → GREEN) +- [ ] Vérifier que le test existant `AuthTest` n'est pas cassé + +--- + +## 🟡 Priorité MOYENNE + +### 2. HSTS Header en production + +**Risque :** Sans `Strict-Transport-Security`, un attaquant peut forcer une connexion HTTP. + +- [ ] Ajouter dans `SecurityHeadersListener.php` : `Strict-Transport-Security: max-age=31536000; includeSubDomains` +- [ ] Conditionner à l'env `prod` uniquement (pas en dev/test — HTTPS non disponible) +- [ ] Écrire un test : vérifier que le header est présent sur une réponse API en env `prod` + +--- + +### 3. Logging des tentatives d'authentification échouées + +**Risque :** Aucune traçabilité des accès refusés — impossible de détecter une attaque a posteriori. + +- [ ] Créer `AuthenticationFailureListener` écoutant `lexik_jwt_authentication.on_authentication_failure` +- [ ] Logger : email tenté, IP client, timestamp, user-agent +- [ ] Logger également les `AccessDeniedHttpException` dans un `ExceptionListener` dédié (ou l'existant) +- [ ] Enregistrer le listener dans `services.yaml` avec le tag event +- [ ] Écrire un test : vérifier que le logger est appelé lors d'un login échoué + +--- + +### 4. `composer audit` dans le pipeline CI/CD + +**Risque :** Une dépendance vulnérable peut passer inaperçue sans vérification automatique. + +- [ ] Ajouter un step dans `.github/workflows/ci.yml` : + ```yaml + - name: 🔍 Audit des dépendances + run: composer audit + ``` +- [ ] Placer ce step avant les tests (fail fast) +- [ ] Documenter dans `avancement.md` que le pipeline intègre l'audit de dépendances + +--- + +## 🟢 Priorité BASSE + +### 5. Contraintes `Assert` sur les DTOs / ApiResource inputs + +**Risque :** Faible (validation métier déjà en place dans les Services), mais les messages d'erreur sont moins standardisés qu'avec le Symfony Validator. + +- [ ] Identifier les champs éditables via PATCH dans les entités `File`, `Folder`, `User` +- [ ] Ajouter `#[Assert\NotBlank]`, `#[Assert\Length(max: 255)]` sur les champs texte +- [ ] Ajouter `#[Assert\Email]` sur `User::$email` +- [ ] Vérifier que API Platform retourne des erreurs 422 avec violations détaillées +- [ ] Écrire des tests : payload invalide → 422 avec message explicite + +--- + +## 📋 Résumé + +| # | Tâche | Priorité | Difficulté estimée | +|---|-------|----------|--------------------| +| 1 | Rate limiting login | 🔴 Haute | Facile (bundle natif Symfony) | +| 2 | HSTS header | 🟡 Moyenne | Trivial (1 ligne de code) | +| 3 | Auth failure logging | 🟡 Moyenne | Facile (listener + PSR-3) | +| 4 | `composer audit` en CI | 🟡 Moyenne | Trivial (1 step YAML) | +| 5 | Assert sur DTOs | 🟢 Basse | Moyen (plusieurs entités) | diff --git a/composer.json b/composer.json index e4e06a9d..b309d685 100644 --- a/composer.json +++ b/composer.json @@ -30,6 +30,7 @@ "symfony/mime": "8.0.*", "symfony/property-access": "8.0.*", "symfony/property-info": "8.0.*", + "symfony/rate-limiter": "8.0.*", "symfony/runtime": "8.0.*", "symfony/security-bundle": "8.0.*", "symfony/serializer": "8.0.*", diff --git a/composer.lock b/composer.lock index 9c7c9f63..a595e318 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": "c580fad70a9e88a56dd6907b176b2414", + "content-hash": "d98c39e57148c18497563d2c817f2862", "packages": [ { "name": "api-platform/doctrine-common", @@ -6594,6 +6594,80 @@ ], "time": "2026-02-13T12:14:15+00:00" }, + { + "name": "symfony/rate-limiter", + "version": "v8.0.7", + "source": { + "type": "git", + "url": "https://github.com/symfony/rate-limiter.git", + "reference": "1f8159c50b55e78810f5a8f60889d0b6b3a11deb" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/symfony/rate-limiter/zipball/1f8159c50b55e78810f5a8f60889d0b6b3a11deb", + "reference": "1f8159c50b55e78810f5a8f60889d0b6b3a11deb", + "shasum": "" + }, + "require": { + "php": ">=8.4", + "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.0.7" + }, + "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-03-04T13:55:34+00:00" + }, { "name": "symfony/routing", "version": "v8.0.6", diff --git a/config/packages/security.yaml b/config/packages/security.yaml index 61a6e5d0..51939aac 100644 --- a/config/packages/security.yaml +++ b/config/packages/security.yaml @@ -22,6 +22,9 @@ security: password_path: password success_handler: lexik_jwt_authentication.handler.authentication_success failure_handler: lexik_jwt_authentication.handler.authentication_failure + login_throttling: + max_attempts: 5 + interval: '15 minutes' api: pattern: ^/api diff --git a/config/reference.php b/config/reference.php index 8b76720f..5a10a144 100644 --- a/config/reference.php +++ b/config/reference.php @@ -622,7 +622,7 @@ * }>, * }, * rate_limiter?: bool|array{ // Rate limiter configuration - * enabled?: bool|Param, // Default: false + * enabled?: bool|Param, // Default: true * limiters?: arraygetRequest(); + + $this->logger->warning('Authentication failure', [ + 'email' => $request->getPayload()->getString('email'), + 'ip' => $request->getClientIp(), + 'user_agent' => $request->headers->get('User-Agent'), + 'exception' => $event->getException()->getMessageKey(), + ]); + } +} diff --git a/src/EventListener/LoginThrottlingFailureListener.php b/src/EventListener/LoginThrottlingFailureListener.php new file mode 100644 index 00000000..754cfa20 --- /dev/null +++ b/src/EventListener/LoginThrottlingFailureListener.php @@ -0,0 +1,31 @@ +getException() instanceof TooManyLoginAttemptsAuthenticationException) { + return; + } + + $event->setResponse(new JsonResponse( + ['code' => Response::HTTP_TOO_MANY_REQUESTS, 'message' => 'Too many login attempts. Please try again later.'], + Response::HTTP_TOO_MANY_REQUESTS + )); + } +} diff --git a/src/EventListener/SecurityHeadersListener.php b/src/EventListener/SecurityHeadersListener.php index e8716fa1..ea2fc397 100644 --- a/src/EventListener/SecurityHeadersListener.php +++ b/src/EventListener/SecurityHeadersListener.php @@ -12,9 +12,10 @@ * Ajoute les headers de sécurité HTTP sur toutes les réponses de l'API. * * Headers ajoutés sur toutes les réponses : - * - X-Content-Type-Options: nosniff → empêche le MIME sniffing navigateur - * - X-Frame-Options: DENY → empêche l'embedding dans des iframes (clickjacking) - * - Referrer-Policy: no-referrer → ne transmet pas l'URL de la page précédente + * - X-Content-Type-Options: nosniff → empêche le MIME sniffing navigateur + * - X-Frame-Options: DENY → empêche l'embedding dans des iframes (clickjacking) + * - Referrer-Policy: no-referrer → ne transmet pas l'URL de la page précédente + * - Strict-Transport-Security (prod only) → force HTTPS, bloque le SSL-stripping * * Header ajouté uniquement sur les routes /api/* (hors /api/docs) : * - Content-Security-Policy: default-src 'none' → l'API ne sert que du JSON @@ -22,6 +23,10 @@ #[AsEventListener(event: KernelEvents::RESPONSE)] final class SecurityHeadersListener { + public function __construct(private readonly string $env) + { + } + public function __invoke(ResponseEvent $event): void { if (!$event->isMainRequest()) { @@ -33,6 +38,11 @@ public function __invoke(ResponseEvent $event): void $headers->set('X-Frame-Options', 'DENY'); $headers->set('Referrer-Policy', 'no-referrer'); + // HSTS : uniquement en prod — HTTPS non disponible en dev/test. + if ('prod' === $this->env) { + $headers->set('Strict-Transport-Security', 'max-age=31536000; includeSubDomains'); + } + $path = $event->getRequest()->getPathInfo(); // CSP strict uniquement sur l'API (JSON) — pas sur le frontend HTML. diff --git a/tests/Api/AuthTest.php b/tests/Api/AuthTest.php index 05513332..d96a7af7 100644 --- a/tests/Api/AuthTest.php +++ b/tests/Api/AuthTest.php @@ -42,6 +42,11 @@ protected function setUp(): void $conn->executeStatement('DELETE FROM users'); $conn->executeStatement('SET FOREIGN_KEY_CHECKS=1'); $this->em->clear(); + + // Réinitialise les compteurs de rate limiting entre chaque test + if (static::getContainer()->has('cache.rate_limiter')) { + static::getContainer()->get('cache.rate_limiter')->clear(); + } } private function createUserWithPassword(string $email, string $plainPassword): User @@ -179,4 +184,26 @@ public function testRefreshReturns401WithExpiredToken(): void $this->assertResponseStatusCodeSame(401); } + + public function testLoginIsThrottledAfterFiveFailures(): void + { + $this->createUserWithPassword('throttle@example.com', 'password123'); + + $client = static::createClient(); + + // 5 tentatives échouées + for ($i = 0; $i < 5; $i++) { + $client->request('POST', '/api/v1/auth/login', [ + 'json' => ['email' => 'throttle@example.com', 'password' => 'wrongpassword'], + ]); + $this->assertResponseStatusCodeSame(401); + } + + // La 6ème tentative doit être bloquée par le rate limiter + $client->request('POST', '/api/v1/auth/login', [ + 'json' => ['email' => 'throttle@example.com', 'password' => 'wrongpassword'], + ]); + + $this->assertResponseStatusCodeSame(429); + } } diff --git a/tests/Unit/EventListener/AuthenticationFailureListenerTest.php b/tests/Unit/EventListener/AuthenticationFailureListenerTest.php new file mode 100644 index 00000000..41546e88 --- /dev/null +++ b/tests/Unit/EventListener/AuthenticationFailureListenerTest.php @@ -0,0 +1,99 @@ +logger = $this->createMock(LoggerInterface::class); + $this->listener = new AuthenticationFailureListener($this->logger); + } + + private function buildEvent(string $email = 'test@example.com', string $userAgent = 'TestAgent/1.0'): LoginFailureEvent + { + $request = Request::create( + '/api/v1/auth/login', + 'POST', + [], + [], + [], + [ + 'HTTP_USER_AGENT' => $userAgent, + 'REMOTE_ADDR' => '192.168.1.1', + 'CONTENT_TYPE' => 'application/json', + ], + json_encode(['email' => $email, 'password' => 'wrongpassword']) + ); + + $authenticator = $this->createMock(AuthenticatorInterface::class); + $exception = new BadCredentialsException(); + + return new LoginFailureEvent($exception, $authenticator, $request, null, 'login'); + } + + public function testLogsWarningOnFailedLogin(): void + { + $this->logger + ->expects($this->once()) + ->method('warning') + ->with( + $this->stringContains('Authentication failure'), + $this->callback(fn (array $context) => true) + ); + + ($this->listener)($this->buildEvent()); + } + + public function testLogsEmailInContext(): void + { + $this->logger + ->expects($this->once()) + ->method('warning') + ->with( + $this->anything(), + $this->callback(fn (array $context) => 'attacker@evil.com' === $context['email']) + ); + + ($this->listener)($this->buildEvent(email: 'attacker@evil.com')); + } + + public function testLogsIpInContext(): void + { + $this->logger + ->expects($this->once()) + ->method('warning') + ->with( + $this->anything(), + $this->callback(fn (array $context) => '192.168.1.1' === $context['ip']) + ); + + ($this->listener)($this->buildEvent()); + } + + public function testLogsUserAgentInContext(): void + { + $this->logger + ->expects($this->once()) + ->method('warning') + ->with( + $this->anything(), + $this->callback(fn (array $context) => 'CustomBot/2.0' === $context['user_agent']) + ); + + ($this->listener)($this->buildEvent(userAgent: 'CustomBot/2.0')); + } +} diff --git a/tests/Unit/EventListener/SecurityHeadersListenerTest.php b/tests/Unit/EventListener/SecurityHeadersListenerTest.php new file mode 100644 index 00000000..d0638b62 --- /dev/null +++ b/tests/Unit/EventListener/SecurityHeadersListenerTest.php @@ -0,0 +1,78 @@ +createMock(HttpKernelInterface::class); + $request = Request::create($path); + $response = new Response(); + + return new ResponseEvent($kernel, $request, HttpKernelInterface::MAIN_REQUEST, $response); + } + + public function testCommonSecurityHeadersAreAlwaysPresent(): void + { + $event = $this->buildEvent(); + (new SecurityHeadersListener('test'))($event); + + $headers = $event->getResponse()->headers; + $this->assertSame('nosniff', $headers->get('X-Content-Type-Options')); + $this->assertSame('DENY', $headers->get('X-Frame-Options')); + $this->assertSame('no-referrer', $headers->get('Referrer-Policy')); + } + + public function testCspHeaderIsSetOnApiRoutes(): void + { + $event = $this->buildEvent('/api/v1/users'); + (new SecurityHeadersListener('test'))($event); + + $this->assertSame("default-src 'none'", $event->getResponse()->headers->get('Content-Security-Policy')); + } + + public function testCspHeaderIsNotSetOnApiDocs(): void + { + $event = $this->buildEvent('/api/docs'); + (new SecurityHeadersListener('test'))($event); + + $this->assertNull($event->getResponse()->headers->get('Content-Security-Policy')); + } + + public function testHstsHeaderIsPresentInProd(): void + { + $event = $this->buildEvent('/api/v1/users'); + (new SecurityHeadersListener('prod'))($event); + + $this->assertSame( + 'max-age=31536000; includeSubDomains', + $event->getResponse()->headers->get('Strict-Transport-Security') + ); + } + + public function testHstsHeaderIsAbsentInDev(): void + { + $event = $this->buildEvent('/api/v1/users'); + (new SecurityHeadersListener('dev'))($event); + + $this->assertNull($event->getResponse()->headers->get('Strict-Transport-Security')); + } + + public function testHstsHeaderIsAbsentInTest(): void + { + $event = $this->buildEvent('/api/v1/users'); + (new SecurityHeadersListener('test'))($event); + + $this->assertNull($event->getResponse()->headers->get('Strict-Transport-Security')); + } +}