fix(audit-log) : applique fixes code review PR #9
Resout les 5 findings de la review automatique + couverture ManyToMany annoncee dans CLAUDE.md : - AuditListener : resolution de la classe via ClassMetadata plutot que `$entity::class` direct (defense proxy Doctrine : sous ORM 2 les lazies sont des `Proxies\__CG__\...`). Test de regression via getReference(). - AuditListener : capture des modifications de collections to-many (OneToMany / ManyToMany) via getScheduledCollectionUpdates / getScheduledCollectionDeletions. Les diffs sont mergees dans le changeset existant ou creent une entree "update" dediee. - AuditLogResource + Provider : filtre multi-valeurs `entity_type[]=X&entity_type[]=Y` (IN clause DBAL via ArrayParameterType::STRING), endpoint `/audit-log-entity-types` pour alimenter le MalioSelectCheckbox cote front. - audit-log.vue : refonte complete. Passage a `MalioDataTable`, composants `Malio*` (MalioInputText, MalioSelectCheckbox, MalioButton), suppression complete de la persistance URL (`readQuery` / `syncQuery` / `route.query`). `datetime-local` conserve avec TODO pointant l'exception CLAUDE.md. - AuditTimeline : fix du saut d'items 11-30. `PAGE_SIZE = 10` aligne avec un `itemsPerPage=10` passe au backend. Token anti-race pour ignorer les reponses tardives quand l'entite affichee change. - AuditLogDetail : affichage des diffs de collections to-many (+ / -) dans le tableau field/old/new existant. - logout.vue : ajout du `resetAuditLog()` au logout pour eviter qu'un user suivant (meme onglet) voie l'etat audit de l'ancien. - Permission / Role / Site : marquage `#[Auditable]`. - Version bump 0.1.32 → 0.1.34. Tests : 228 / 228 (221 assertions → 851, dont regressions proxy + M2M). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Matthieu
@@ -95,13 +95,6 @@ final class AuditLogApiTest extends AbstractApiTestCase
|
||||
self::assertArrayHasKey('totalItems', $data);
|
||||
}
|
||||
|
||||
/**
|
||||
* Le frontend force `Accept: application/ld+json` dans `useAuditLog` pour
|
||||
* recuperer les cles prefixees `hydra:*` (et `hydra:view` pour la
|
||||
* pagination). Ce test verrouille ce contrat : sans lui, un changement
|
||||
* de configuration API Platform cassant le JSON-LD passerait inaperçu
|
||||
* et le tableau admin apparaitrait silencieusement vide en production.
|
||||
*/
|
||||
/**
|
||||
* Le frontend demande explicitement `application/ld+json` dans `useAuditLog`
|
||||
* pour obtenir l'objet Hydra complet (`member`, `totalItems`, `view`). Sous
|
||||
@@ -206,6 +199,139 @@ final class AuditLogApiTest extends AbstractApiTestCase
|
||||
self::assertContains($response->getStatusCode(), [404, 405], 'POST doit etre refuse (pas d\'operation d\'ecriture exposee)');
|
||||
}
|
||||
|
||||
/**
|
||||
* Filtre multi-valeurs `entity_type[]=X&entity_type[]=Y` : l'union des
|
||||
* deux types est retournee. On seed 2 types differents (core.User et
|
||||
* core.Role) et on verifie que les deux apparaissent sous notre runTag,
|
||||
* et qu'une valeur non existante (`core.Nonexistent`) n'ajoute rien.
|
||||
*
|
||||
* On interroge avec itemsPerPage=100 pour englober nos 5 lignes quel
|
||||
* que soit le bruit de lignes preexistantes dans audit_log.
|
||||
*/
|
||||
public function testFilterByMultipleEntityTypes(): void
|
||||
{
|
||||
// Seed 2 lignes supplementaires avec un autre entity_type.
|
||||
$this->seedExtraRow('core.Role', '1001', 'create');
|
||||
$this->seedExtraRow('core.Role', '1002', 'update');
|
||||
|
||||
$client = $this->authenticatedClient('admin', 'admin');
|
||||
$response = $client->request('GET', '/api/audit-logs?'.http_build_query([
|
||||
'entity_type' => ['core.User', 'core.Role', 'core.Nonexistent'],
|
||||
'itemsPerPage' => 100,
|
||||
]));
|
||||
|
||||
self::assertSame(200, $response->getStatusCode());
|
||||
$data = $response->toArray();
|
||||
|
||||
// Filtre sur notre runTag pour isoler nos 5 lignes (3 User + 2 Role)
|
||||
// independamment des entrees pre-existantes de la table.
|
||||
$ours = array_values(array_filter(
|
||||
$data['member'],
|
||||
fn (array $m) => ($m['requestId'] ?? null) === $this->runTag,
|
||||
));
|
||||
self::assertCount(5, $ours, 'Les 3 lignes core.User + 2 lignes core.Role doivent etre retournees.');
|
||||
|
||||
$types = array_unique(array_map(fn (array $m) => $m['entityType'], $ours));
|
||||
sort($types);
|
||||
self::assertSame(['core.Role', 'core.User'], $types);
|
||||
|
||||
// Verifier qu'aucune ligne hors filtre n'apparait dans la reponse.
|
||||
foreach ($data['member'] as $member) {
|
||||
self::assertContains($member['entityType'], ['core.User', 'core.Role']);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Recherche partielle insensible a la casse sur `performed_by` via ILIKE.
|
||||
* Le seed utilise `performed_by=admin` ; on cherche `ADM` pour tester
|
||||
* a la fois la casse et le wildcard contains.
|
||||
*/
|
||||
public function testFilterByPerformedByPartialMatch(): void
|
||||
{
|
||||
$client = $this->authenticatedClient('admin', 'admin');
|
||||
$response = $client->request('GET', '/api/audit-logs?performed_by=ADM&entity_id=999');
|
||||
|
||||
self::assertSame(200, $response->getStatusCode());
|
||||
$data = $response->toArray();
|
||||
$ours = array_filter($data['member'], fn (array $m) => ($m['requestId'] ?? null) === $this->runTag);
|
||||
self::assertGreaterThan(0, count($ours), 'La recherche ILIKE doit matcher "ADM" -> "admin".');
|
||||
}
|
||||
|
||||
/**
|
||||
* Les caracteres wildcard PostgreSQL (`%`, `_`) saisis par l'utilisateur
|
||||
* doivent etre echappes et traites comme caracteres litteraux, pas comme
|
||||
* des metacaracteres LIKE. Idem pour le backslash qui doit etre double
|
||||
* pour ne pas interferer avec la clause ESCAPE.
|
||||
*/
|
||||
public function testFilterByPerformedByEscapesWildcards(): void
|
||||
{
|
||||
$client = $this->authenticatedClient('admin', 'admin');
|
||||
|
||||
// `%` seul doit matcher 0 ligne (personne n'a `%` dans performed_by).
|
||||
$response = $client->request('GET', '/api/audit-logs?performed_by=%25&entity_id=999');
|
||||
self::assertSame(200, $response->getStatusCode());
|
||||
$data = $response->toArray();
|
||||
$ours = array_filter($data['member'], fn (array $m) => ($m['requestId'] ?? null) === $this->runTag);
|
||||
self::assertCount(0, $ours, '% doit etre traite comme literal, pas wildcard.');
|
||||
|
||||
// `_` seul (wildcard single-char en LIKE) doit aussi matcher 0 ligne.
|
||||
$response = $client->request('GET', '/api/audit-logs?performed_by=_&entity_id=999');
|
||||
self::assertSame(200, $response->getStatusCode());
|
||||
$data = $response->toArray();
|
||||
$ours = array_filter($data['member'], fn (array $m) => ($m['requestId'] ?? null) === $this->runTag);
|
||||
self::assertCount(0, $ours, '_ doit etre traite comme literal, pas wildcard single-char.');
|
||||
|
||||
// `\` (backslash) dans le motif ne doit pas casser la clause ESCAPE :
|
||||
// on attend une reponse 200 (pas 500), meme si le resultat est vide.
|
||||
$response = $client->request('GET', '/api/audit-logs?performed_by=%5C&entity_id=999');
|
||||
self::assertSame(200, $response->getStatusCode(), 'Un backslash dans le filtre ne doit pas produire de 500.');
|
||||
}
|
||||
|
||||
/**
|
||||
* L'endpoint `/api/audit-log-entity-types` retourne la liste des valeurs
|
||||
* distinctes de `entity_type` presentes dans la table. La presence du
|
||||
* seed runTag garantit au moins `core.User`.
|
||||
*/
|
||||
public function testEntityTypesEndpointReturnsDistinctTypes(): void
|
||||
{
|
||||
$client = $this->authenticatedClient('admin', 'admin');
|
||||
$response = $client->request('GET', '/api/audit-log-entity-types');
|
||||
|
||||
self::assertSame(200, $response->getStatusCode());
|
||||
$data = $response->toArray();
|
||||
self::assertArrayHasKey('entityTypes', $data);
|
||||
self::assertIsArray($data['entityTypes']);
|
||||
self::assertContains('core.User', $data['entityTypes']);
|
||||
}
|
||||
|
||||
public function testEntityTypesEndpointRequiresPermission(): void
|
||||
{
|
||||
$credentials = $this->createUserWithPermission('core.users.view');
|
||||
$client = $this->authenticatedClient($credentials['username'], $credentials['password']);
|
||||
$response = $client->request('GET', '/api/audit-log-entity-types');
|
||||
|
||||
self::assertSame(403, $response->getStatusCode());
|
||||
}
|
||||
|
||||
/**
|
||||
* Helper interne pour seeder une ligne additionnelle avec un entity_type
|
||||
* arbitraire, taggee runTag pour nettoyage en tearDown.
|
||||
*/
|
||||
private function seedExtraRow(string $entityType, string $entityId, string $action): void
|
||||
{
|
||||
$this->auditConnection->insert('audit_log', [
|
||||
'id' => Uuid::v7()->toRfc4122(),
|
||||
'entity_type' => $entityType,
|
||||
'entity_id' => $entityId,
|
||||
'action' => $action,
|
||||
'changes' => json_encode(['field' => ['old' => 1, 'new' => 2]], JSON_THROW_ON_ERROR),
|
||||
'performed_by' => 'admin',
|
||||
'performed_at' => new DateTimeImmutable('now', new DateTimeZone('UTC'))->format('Y-m-d H:i:sO'),
|
||||
'ip_address' => null,
|
||||
'request_id' => $this->runTag,
|
||||
]);
|
||||
}
|
||||
|
||||
/**
|
||||
* Insere 3 lignes temoins taggees avec le runTag pour un nettoyage sur.
|
||||
*/
|
||||
@@ -220,7 +346,10 @@ final class AuditLogApiTest extends AbstractApiTestCase
|
||||
'action' => 'update',
|
||||
'changes' => ['isAdmin' => ['old' => false, 'new' => true]],
|
||||
'performed_by' => 'admin',
|
||||
'performed_at' => $now->modify('-2 hours'),
|
||||
// Offsets faibles (secondes) : garantit que les 3 lignes
|
||||
// restent parmi les plus recentes de audit_log meme quand la
|
||||
// table contient plusieurs centaines de lignes historiques.
|
||||
'performed_at' => $now->modify('-2 seconds'),
|
||||
],
|
||||
[
|
||||
'entity_type' => 'core.User',
|
||||
@@ -228,7 +357,7 @@ final class AuditLogApiTest extends AbstractApiTestCase
|
||||
'action' => 'update',
|
||||
'changes' => ['username' => ['old' => 'x', 'new' => 'y']],
|
||||
'performed_by' => 'admin',
|
||||
'performed_at' => $now->modify('-1 hour'),
|
||||
'performed_at' => $now->modify('-1 second'),
|
||||
],
|
||||
[
|
||||
'entity_type' => 'core.User',
|
||||
|
||||
@@ -4,6 +4,8 @@ declare(strict_types=1);
|
||||
|
||||
namespace App\Tests\Module\Core\Infrastructure\Doctrine;
|
||||
|
||||
use App\Module\Core\Domain\Entity\Permission;
|
||||
use App\Module\Core\Domain\Entity\Role;
|
||||
use App\Module\Core\Domain\Entity\User;
|
||||
use Doctrine\DBAL\Connection;
|
||||
use Doctrine\ORM\EntityManagerInterface;
|
||||
@@ -32,6 +34,9 @@ final class AuditListenerTest extends KernelTestCase
|
||||
/** @var list<int> IDs de users crees par le test (nettoyage en tearDown) */
|
||||
private array $createdUserIds = [];
|
||||
|
||||
/** @var list<int> IDs de roles crees par le test (nettoyage en tearDown) */
|
||||
private array $createdRoleIds = [];
|
||||
|
||||
private string $testRunTag;
|
||||
|
||||
protected function setUp(): void
|
||||
@@ -66,6 +71,24 @@ final class AuditListenerTest extends KernelTestCase
|
||||
$this->em->flush();
|
||||
}
|
||||
|
||||
if ([] !== $this->createdRoleIds) {
|
||||
foreach ($this->createdRoleIds as $id) {
|
||||
$role = $this->em->find(Role::class, $id);
|
||||
if (null !== $role) {
|
||||
$this->em->remove($role);
|
||||
}
|
||||
}
|
||||
$this->em->flush();
|
||||
// Nettoie egalement les lignes audit de ces roles (entity_id est
|
||||
// une colonne text, on delete en boucle pour simplifier le binding).
|
||||
foreach ($this->createdRoleIds as $id) {
|
||||
$this->auditConnection->executeStatement(
|
||||
'DELETE FROM audit_log WHERE entity_type = \'core.Role\' AND entity_id = :id',
|
||||
['id' => (string) $id],
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
$this->auditConnection->executeStatement(
|
||||
"DELETE FROM audit_log WHERE entity_type = 'core.User' AND changes->>'username' LIKE :tag",
|
||||
['tag' => $this->testRunTag.'%'],
|
||||
@@ -154,6 +177,157 @@ final class AuditListenerTest extends KernelTestCase
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Regression test : une entite recuperee via `getReference()` (proxy /
|
||||
* ghost object lazy) doit etre auditee avec le FQCN canonique. Sur
|
||||
* Doctrine ORM 3 + PHP 8.4, les lazy ghosts preservent `::class` reel
|
||||
* — mais sous Doctrine 2 ou en cas de retour a un `__CG__\` proxy,
|
||||
* l'audit doit toujours resoudre la classe via `ClassMetadata` et
|
||||
* jamais aboutir a un `entity_type` de type `Proxies\__CG__\...\User`.
|
||||
*/
|
||||
public function testLogsUpdateOnProxyEntity(): void
|
||||
{
|
||||
$user = $this->makeUser();
|
||||
$this->em->persist($user);
|
||||
$this->em->flush();
|
||||
$userId = (int) $user->getId();
|
||||
$this->createdUserIds[] = $userId;
|
||||
|
||||
// Detache puis recupere via getReference : sur Doctrine 2, renvoie
|
||||
// un `Proxies\__CG__\...\User` ; sur Doctrine 3 + PHP 8.4 le ghost
|
||||
// object reste instance de la classe reelle — dans tous les cas la
|
||||
// resolution via ClassMetadata doit produire un audit correct.
|
||||
$this->em->clear();
|
||||
|
||||
$proxy = $this->em->getReference(User::class, $userId);
|
||||
self::assertNotNull($proxy);
|
||||
|
||||
// Reset de la baseline : on ne garde que la ligne update du proxy.
|
||||
$this->auditConnection->executeStatement(
|
||||
'DELETE FROM audit_log WHERE entity_id = :id AND entity_type = \'core.User\'',
|
||||
['id' => (string) $userId],
|
||||
);
|
||||
|
||||
$proxy->setIsAdmin(true);
|
||||
$this->em->flush();
|
||||
|
||||
$rows = $this->fetchAuditRows($userId);
|
||||
self::assertCount(1, $rows, 'La mutation sur un proxy doit etre auditee.');
|
||||
self::assertSame('update', $rows[0]['action']);
|
||||
// L'entity_type doit etre le FQCN canonique, pas celui du proxy.
|
||||
self::assertSame('core.User', $rows[0]['entity_type']);
|
||||
}
|
||||
|
||||
/**
|
||||
* Verifie que l'ajout d'une permission a un role est bien audite sous
|
||||
* la forme `{permissions: {added: [id], removed: []}}`. Regression test
|
||||
* pour le bug "ManyToMany collections ignorees par getEntityChangeSet".
|
||||
*/
|
||||
public function testLogsManyToManyCollectionAddition(): void
|
||||
{
|
||||
$roleCode = 'audittest_'.bin2hex(random_bytes(3));
|
||||
$role = new Role($roleCode, 'Test role '.$roleCode);
|
||||
$this->em->persist($role);
|
||||
$this->em->flush();
|
||||
$roleId = (int) $role->getId();
|
||||
$this->createdRoleIds[] = $roleId;
|
||||
|
||||
// Reset baseline : on ne veut que le log de l'update de collection.
|
||||
$this->auditConnection->executeStatement(
|
||||
'DELETE FROM audit_log WHERE entity_type = \'core.Role\' AND entity_id = :id',
|
||||
['id' => (string) $roleId],
|
||||
);
|
||||
|
||||
// Recupere une permission existante (fixtures garantissent core.users.view).
|
||||
$permission = $this->em->getRepository(Permission::class)->findOneBy(['code' => 'core.users.view']);
|
||||
self::assertNotNull($permission, 'Fixture core.users.view manquante.');
|
||||
|
||||
$role->addPermission($permission);
|
||||
$this->em->flush();
|
||||
|
||||
$rows = $this->fetchRoleAuditRows($roleId);
|
||||
self::assertCount(1, $rows, 'Une ligne update attendue pour l\'ajout de permission.');
|
||||
self::assertSame('update', $rows[0]['action']);
|
||||
|
||||
$changes = json_decode($rows[0]['changes'], true, 512, JSON_THROW_ON_ERROR);
|
||||
self::assertArrayHasKey('permissions', $changes, 'Le changeset doit contenir le champ "permissions".');
|
||||
self::assertSame([], $changes['permissions']['removed']);
|
||||
self::assertSame([(int) $permission->getId()], $changes['permissions']['added']);
|
||||
}
|
||||
|
||||
/**
|
||||
* Symetrique : retirer une permission d'un role est audite sous
|
||||
* `{permissions: {added: [], removed: [id]}}`.
|
||||
*/
|
||||
public function testLogsManyToManyCollectionRemoval(): void
|
||||
{
|
||||
$permission = $this->em->getRepository(Permission::class)->findOneBy(['code' => 'core.users.view']);
|
||||
self::assertNotNull($permission);
|
||||
|
||||
$roleCode = 'audittest_'.bin2hex(random_bytes(3));
|
||||
$role = new Role($roleCode, 'Test role '.$roleCode);
|
||||
$role->addPermission($permission);
|
||||
$this->em->persist($role);
|
||||
$this->em->flush();
|
||||
$roleId = (int) $role->getId();
|
||||
$this->createdRoleIds[] = $roleId;
|
||||
|
||||
// Reset baseline.
|
||||
$this->auditConnection->executeStatement(
|
||||
'DELETE FROM audit_log WHERE entity_type = \'core.Role\' AND entity_id = :id',
|
||||
['id' => (string) $roleId],
|
||||
);
|
||||
|
||||
$role->removePermission($permission);
|
||||
$this->em->flush();
|
||||
|
||||
$rows = $this->fetchRoleAuditRows($roleId);
|
||||
self::assertCount(1, $rows);
|
||||
$changes = json_decode($rows[0]['changes'], true, 512, JSON_THROW_ON_ERROR);
|
||||
self::assertSame([], $changes['permissions']['added']);
|
||||
self::assertSame([(int) $permission->getId()], $changes['permissions']['removed']);
|
||||
}
|
||||
|
||||
/**
|
||||
* Regression test : supprimer un role avec des permissions attachees doit
|
||||
* preserver la liste des permissions dans le snapshot delete. C'etait le
|
||||
* trou principal du fix ManyToMany initial (reviewer Codex round 2).
|
||||
*/
|
||||
public function testDeleteSnapshotIncludesManyToManyIds(): void
|
||||
{
|
||||
$permission = $this->em->getRepository(Permission::class)->findOneBy(['code' => 'core.users.view']);
|
||||
self::assertNotNull($permission);
|
||||
|
||||
$roleCode = 'audittest_'.bin2hex(random_bytes(3));
|
||||
$role = new Role($roleCode, 'Delete test '.$roleCode);
|
||||
$role->addPermission($permission);
|
||||
$this->em->persist($role);
|
||||
$this->em->flush();
|
||||
$roleId = (int) $role->getId();
|
||||
|
||||
$this->em->remove($role);
|
||||
$this->em->flush();
|
||||
|
||||
$rows = $this->fetchRoleAuditRows($roleId);
|
||||
// create + update (permission ajoutee) + delete attendus.
|
||||
$actions = array_column($rows, 'action');
|
||||
self::assertContains('delete', $actions);
|
||||
|
||||
$deleteRow = $rows[array_search('delete', $actions, true)];
|
||||
$changes = json_decode($deleteRow['changes'], true, 512, JSON_THROW_ON_ERROR);
|
||||
|
||||
// Le snapshot delete doit contenir la liste des IDs de permissions
|
||||
// attachees au role au moment de la suppression.
|
||||
self::assertArrayHasKey('permissions', $changes);
|
||||
self::assertSame([(int) $permission->getId()], $changes['permissions']);
|
||||
|
||||
// Nettoyage manuel (le role est deja supprime, on ne peut plus passer par $this->em).
|
||||
$this->auditConnection->executeStatement(
|
||||
'DELETE FROM audit_log WHERE entity_type = \'core.Role\' AND entity_id = :id',
|
||||
['id' => (string) $roleId],
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* @return list<array{id: string, entity_type: string, entity_id: string, action: string, changes: string}>
|
||||
*/
|
||||
@@ -178,4 +352,16 @@ final class AuditListenerTest extends KernelTestCase
|
||||
|
||||
return $user;
|
||||
}
|
||||
|
||||
/**
|
||||
* @return list<array{id: string, entity_type: string, entity_id: string, action: string, changes: string}>
|
||||
*/
|
||||
private function fetchRoleAuditRows(int $roleId): array
|
||||
{
|
||||
// @var list<array{id: string, entity_type: string, entity_id: string, action: string, changes: string}> $rows
|
||||
return $this->auditConnection->fetchAllAssociative(
|
||||
'SELECT id, entity_type, entity_id, action, changes FROM audit_log WHERE entity_type = :type AND entity_id = :id ORDER BY performed_at ASC',
|
||||
['type' => 'core.Role', 'id' => (string) $roleId],
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user