From a88cb1bc351bf1a259374c5bba5df798e4ff57fd Mon Sep 17 00:00:00 2001
From: Matthieu <contact@malio.fr>
Date: Fri, 19 Jun 2026 22:39:26 +0200
Subject: [PATCH] fix(core) : harden review findings (me-provider null guard,
 audit-ignore plainpassword, rbac self-edit guard, module id dedup, audit
 pagination guard)

---
 frontend/components/admin/AdminAuditTab.vue       |  4 +++-
 src/Module/Core/Domain/Entity/User.php            |  1 +
 .../ApiPlatform/State/MeProvider.php              |  9 +++++++--
 .../State/Processor/UserRbacProcessor.php         | 15 ++++++++++++++-
 src/Shared/Domain/Module/ModuleRegistry.php       |  9 +++++++--
 5 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/frontend/components/admin/AdminAuditTab.vue b/frontend/components/admin/AdminAuditTab.vue
index 351144d..58608f8 100644
--- a/frontend/components/admin/AdminAuditTab.vue
+++ b/frontend/components/admin/AdminAuditTab.vue
@@ -98,7 +98,9 @@ const entityTypeOptions = computed<{ value: string, label: string }[]>(() =>
     entityTypes.value.map((value) => ({ value, label: entityTypeLabel(value) })),
 )
 
-const hasNextPage = computed(() => page.value * PAGE_SIZE < totalItems.value)
+// PAGE_SIZE must match the API default page size. The full-page guard keeps the
+// "next" button accurate even on the last (partial) page.
+const hasNextPage = computed(() => rows.value.length >= PAGE_SIZE && page.value * PAGE_SIZE < totalItems.value)
 
 function entityTypeLabel(value: string): string {
     const key = `audit.entity.${value}`
diff --git a/src/Module/Core/Domain/Entity/User.php b/src/Module/Core/Domain/Entity/User.php
index 6c699f5..8a9dd35 100644
--- a/src/Module/Core/Domain/Entity/User.php
+++ b/src/Module/Core/Domain/Entity/User.php
@@ -94,6 +94,7 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface, SharedU
     private ?string $password = null;
 
     #[Groups(['user:write'])]
+    #[AuditIgnore]
     private ?string $plainPassword = null;
 
     #[ORM\Column(type: Types::DATETIME_IMMUTABLE)]
diff --git a/src/Module/Core/Infrastructure/ApiPlatform/State/MeProvider.php b/src/Module/Core/Infrastructure/ApiPlatform/State/MeProvider.php
index 8e973bb..520e239 100644
--- a/src/Module/Core/Infrastructure/ApiPlatform/State/MeProvider.php
+++ b/src/Module/Core/Infrastructure/ApiPlatform/State/MeProvider.php
@@ -8,6 +8,7 @@ use ApiPlatform\Metadata\Operation;
 use ApiPlatform\State\ProviderInterface;
 use App\Module\Core\Domain\Entity\User;
 use Symfony\Bundle\SecurityBundle\Security;
+use Symfony\Component\HttpKernel\Exception\UnauthorizedHttpException;
 
 /**
  * @implements ProviderInterface<User>
@@ -20,7 +21,11 @@ final readonly class MeProvider implements ProviderInterface
 
     public function provide(Operation $operation, array $uriVariables = [], array $context = []): User
     {
-        // @var User $user
-        return $this->security->getUser();
+        $user = $this->security->getUser();
+        if (!$user instanceof User) {
+            throw new UnauthorizedHttpException('Bearer', 'Not authenticated.');
+        }
+
+        return $user;
     }
 }
diff --git a/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php b/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php
index 4658423..5381e1a 100644
--- a/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php
+++ b/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php
@@ -8,6 +8,8 @@ use ApiPlatform\Metadata\Operation;
 use ApiPlatform\State\ProcessorInterface;
 use App\Module\Core\Domain\Entity\User;
 use Doctrine\ORM\EntityManagerInterface;
+use Symfony\Bundle\SecurityBundle\Security;
+use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
 
 use function assert;
 
@@ -16,12 +18,23 @@ use function assert;
  */
 final readonly class UserRbacProcessor implements ProcessorInterface
 {
-    public function __construct(private EntityManagerInterface $em) {}
+    public function __construct(
+        private EntityManagerInterface $em,
+        private Security $security,
+    ) {}
 
     public function process(mixed $data, Operation $operation, array $uriVariables = [], array $context = []): User
     {
         assert($data instanceof User);
 
+        // Defense-in-depth: a user may never edit their OWN RBAC assignment
+        // through this endpoint, even with core.users.manage — this prevents
+        // self-escalation if the permission is ever delegated to a non-admin.
+        $current = $this->security->getUser();
+        if ($current instanceof User && $current->getId() === $data->getId()) {
+            throw new AccessDeniedHttpException('You cannot edit your own RBAC assignment.');
+        }
+
         $this->em->persist($data);
         $this->em->flush();
 
diff --git a/src/Shared/Domain/Module/ModuleRegistry.php b/src/Shared/Domain/Module/ModuleRegistry.php
index 6c212bb..b0ce7e0 100644
--- a/src/Shared/Domain/Module/ModuleRegistry.php
+++ b/src/Shared/Domain/Module/ModuleRegistry.php
@@ -17,9 +17,14 @@ final class ModuleRegistry
     {
         $ids = [];
         foreach ($moduleClasses as $moduleClass) {
-            if (is_a($moduleClass, ModuleInterface::class, true)) {
-                $ids[] = $moduleClass::id();
+            if (!is_a($moduleClass, ModuleInterface::class, true)) {
+                continue;
             }
+            $id = $moduleClass::id();
+            if (in_array($id, $ids, true)) {
+                throw new InvalidArgumentException(sprintf('Module ID "%s" déclaré plusieurs fois dans la configuration des modules.', $id));
+            }
+            $ids[] = $id;
         }
 
         return $ids;