diff --git a/.github/workflows/phpunit.yml b/.github/workflows/phpunit.yml index a866253..c1746a5 100644 --- a/.github/workflows/phpunit.yml +++ b/.github/workflows/phpunit.yml @@ -15,7 +15,7 @@ jobs: strategy: fail-fast: false matrix: - php: ['8.1', '8.2', '8.3'] + php: ['8.1', '8.2', '8.3', '8.4'] steps: - name: Checkout diff --git a/README.md b/README.md index 132289e..a8fa1df 100644 --- a/README.md +++ b/README.md @@ -78,6 +78,60 @@ data_dog_audit: You can specify either audited or unaudited entities. If both are specified, only audited entities would be taken into account. +### Specify Audited Entities with specific fields (extends version) + +The new configuration has been expanded with fields that can be set as excluded or highlighted. + +```php +class Test { + private string $name; + private bool $noImportant; + private \DateTime $createdAt; + private \DateTime $modifiedAt; +} +``` + +When we only want to audit a few fields: +```yaml +data_dog_audit: + entities: + App\Entity\Test: + mode: include + fields: + - name + - createdAt + - modifiedAt +``` +OR +```yaml +data_dog_audit: + entities: + App\Entity\Test: + mode: exclude + fields: + - noImportant +``` + +The old logic was also included in the new configuration: + +If we want to audit everything on an entity +```yaml +data_dog_audit: + entities: + App\Entity\Test: + mode: include + fields: ~ +``` + +If we want to exclude an entity +```yaml +data_dog_audit: + entities: + App\Entity\Test: + mode: exclude + fields: ~ +``` + ### Impersonation Sometimes, you might also want to blame the `impersonator` user instead of the `impersonated` one. diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index e49255d..4809c94 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -12,8 +12,25 @@ public function getConfigTreeBuilder(): TreeBuilder // @formatter:off $treeBuilder = new TreeBuilder('data_dog_audit'); $treeBuilder->getRootNode() + ->validate() + ->ifTrue(fn ($v) => !empty($v['entities']) && (!empty($v['audited_entities']) || !empty($v['unaudited_entities']))) + ->thenInvalid('If you use the "entities" config you cannot use "audited_entities" and/or "unaudited_entities"') + ->end() + ->children() + ->arrayNode('entities')->canBeUnset()->useAttributeAsKey('key') + ->arrayPrototype() + ->children() + ->enumNode('mode')->values(['include', 'exclude'])->isRequired()->cannotBeEmpty()->end() + ->arrayNode('fields') + ->prototype('scalar')->end() + ->end() + ->end() + ->end() + ->end() + ->end() ->children() ->arrayNode('audited_entities') + ->setDeprecated('data-dog/audit-bundle', 'v1.2', 'Not setting the "%node%" config option is deprecated. Use the "entities" option instead.') ->canBeUnset() ->performNoDeepMerging() ->scalarPrototype()->end() @@ -21,6 +38,7 @@ public function getConfigTreeBuilder(): TreeBuilder ->end() ->children() ->arrayNode('unaudited_entities') + ->setDeprecated('data-dog/audit-bundle', 'v1.2', 'Not setting the "%node%" config option is deprecated. Use the "entities" option instead.') ->canBeUnset() ->performNoDeepMerging() ->scalarPrototype()->end() diff --git a/src/DependencyInjection/DataDogAuditExtension.php b/src/DependencyInjection/DataDogAuditExtension.php index 911b64b..af1daf8 100644 --- a/src/DependencyInjection/DataDogAuditExtension.php +++ b/src/DependencyInjection/DataDogAuditExtension.php @@ -11,7 +11,7 @@ class DataDogAuditExtension extends Extension { public function load(array $configs, ContainerBuilder $container): void { - $loader = new PhpFileLoader($container, new FileLocator(__DIR__ . '/../../config')); + $loader = new PhpFileLoader($container, new FileLocator(__DIR__.'/../../config')); $loader->load('services.php'); $configuration = new Configuration(); @@ -19,14 +19,16 @@ public function load(array $configs, ContainerBuilder $container): void $auditListener = $container->getDefinition('data_dog_audit.listener.audit'); - if (isset($config['audited_entities']) && !empty($config['audited_entities'])) { - $auditListener->addMethodCall('addAuditedEntities', array($config['audited_entities'])); - } else if (isset($config['unaudited_entities'])) { - $auditListener->addMethodCall('addUnauditedEntities', array($config['unaudited_entities'])); + if (isset($config['entities'])) { + $auditListener->addMethodCall('addEntities', [$config['entities']]); + } elseif (isset($config['audited_entities']) && !empty($config['audited_entities'])) { + $auditListener->addMethodCall('addAuditedEntities', [$config['audited_entities']]); + } elseif (isset($config['unaudited_entities'])) { + $auditListener->addMethodCall('addUnauditedEntities', [$config['unaudited_entities']]); } if (isset($config['blame_impersonator'])) { - $auditListener->addMethodCall('setBlameImpersonator', array($config['blame_impersonator'])); + $auditListener->addMethodCall('setBlameImpersonator', [$config['blame_impersonator']]); } } } diff --git a/src/EventListener/AuditListener.php b/src/EventListener/AuditListener.php index ef372d9..066b97a 100644 --- a/src/EventListener/AuditListener.php +++ b/src/EventListener/AuditListener.php @@ -2,6 +2,7 @@ namespace DataDog\AuditBundle\EventListener; +use DataDog\AuditBundle\DBAL\Middleware\AuditFlushMiddleware; use DataDog\AuditBundle\Entity\Association; use DataDog\AuditBundle\Entity\AuditLog; use Doctrine\DBAL\Types\Type; @@ -22,21 +23,23 @@ class AuditListener */ protected $labeler; - protected $auditedEntities = []; + protected array $auditedEntities = []; - protected $unauditedEntities = []; + protected array $unauditedEntities = []; - protected $blameImpersonator = false; + protected array $entities = []; - protected $inserted = []; // [$source, $changeset] + protected bool $blameImpersonator = false; - protected $updated = []; // [$source, $changeset] + protected array $inserted = []; // [$source, $changeset] - protected $removed = []; // [$source, $id] + protected array $updated = []; // [$source, $changeset] - protected $associated = []; // [$source, $target, $mapping] + protected array $removed = []; // [$source, $id] - protected $dissociated = []; // [$source, $target, $id, $mapping] + protected array $associated = []; // [$source, $target, $mapping] + + protected array $dissociated = []; // [$source, $target, $id, $mapping] protected $assocInsertStmt; @@ -49,7 +52,8 @@ class AuditListener public function __construct( private readonly TokenStorageInterface $securityTokenStorage, private readonly EntityManagerInterface $entityManager - ) {} + ) { + } public function setLabeler(?callable $labeler = null): self { @@ -63,7 +67,7 @@ public function getLabeler(): ?callable return $this->labeler; } - public function addAuditedEntities(array $auditedEntities) + public function addAuditedEntities(array $auditedEntities): void { // use entity names as array keys for easier lookup foreach ($auditedEntities as $auditedEntity) { @@ -71,7 +75,7 @@ public function addAuditedEntities(array $auditedEntities) } } - public function addUnauditedEntities(array $unauditedEntities) + public function addUnauditedEntities(array $unauditedEntities): void { // use entity names as array keys for easier lookup foreach ($unauditedEntities as $unauditedEntity) { @@ -79,33 +83,54 @@ public function addUnauditedEntities(array $unauditedEntities) } } - public function setBlameImpersonator($blameImpersonator) + public function addEntities(array $entities): void + { + $this->entities = $entities; + + $auditedEntities = []; + $unauditedEntities = []; + + foreach ($entities as $fqcn => $data) { + if ( + $data['mode'] === 'include' || + ($data['mode'] === 'exclude' && !empty($data['fields'])) + ) { + $auditedEntities[] = $fqcn; + } elseif ($data['mode'] === 'exclude' && empty($data['fields'])) { + $unauditedEntities[] = $fqcn; + } + } + $this->addAuditedEntities($auditedEntities); + $this->addUnauditedEntities($unauditedEntities); + } + + public function setBlameImpersonator(bool $blameImpersonator): void { // blame impersonator user instead of logged user (where applicable) $this->blameImpersonator = $blameImpersonator; } - public function getUnauditedEntities() + public function getUnauditedEntities(): array { return array_keys($this->unauditedEntities); } - protected function isEntityUnaudited($entity) + protected function isEntityUnaudited(object $entity): bool { if (!empty($this->auditedEntities)) { // only selected entities are audited - $isEntityUnaudited = TRUE; + $isEntityUnaudited = true; foreach (array_keys($this->auditedEntities) as $auditedEntity) { if ($entity instanceof $auditedEntity) { - $isEntityUnaudited = FALSE; + $isEntityUnaudited = false; break; } } } else { - $isEntityUnaudited = FALSE; + $isEntityUnaudited = false; foreach (array_keys($this->unauditedEntities) as $unauditedEntity) { if ($entity instanceof $unauditedEntity) { - $isEntityUnaudited = TRUE; + $isEntityUnaudited = true; break; } } @@ -121,7 +146,7 @@ public function onFlush(OnFlushEventArgs $args): void $middlewares = $this->entityManager->getConnection()->getConfiguration()->getMiddlewares(); foreach ($middlewares as $middleware) { - if($middleware::class === 'DataDog\AuditBundle\DBAL\Middleware\AuditFlushMiddleware'){ + if ($middleware::class === AuditFlushMiddleware::class) { $middleware->flushHandler = [$this, 'flush']; } } @@ -188,7 +213,7 @@ public function onFlush(OnFlushEventArgs $args): void } } - public function flush() + public function flush(): void { $uow = $this->entityManager->getUnitOfWork(); @@ -237,7 +262,7 @@ public function flush() $this->dissociated = []; } - protected function associate(EntityManager $em, $source, $target, array $mapping): void + protected function associate(EntityManager $em, ?object $source, ?object $target, array $mapping): void { $this->audit($em, [ 'source' => $this->assoc($em, $source), @@ -249,7 +274,7 @@ protected function associate(EntityManager $em, $source, $target, array $mapping ]); } - protected function dissociate(EntityManager $em, $source, $target, $id, array $mapping): void + protected function dissociate(EntityManager $em, ?object $source, ?object $target, string|int $id, array $mapping): void { $this->audit($em, [ 'source' => $this->assoc($em, $source), @@ -261,7 +286,7 @@ protected function dissociate(EntityManager $em, $source, $target, $id, array $m ]); } - protected function insert(EntityManager $em, $entity, array $ch): void + protected function insert(EntityManager $em, object $entity, array $ch): void { $diff = $this->diff($em, $entity, $ch); if (empty($diff)) { @@ -278,7 +303,7 @@ protected function insert(EntityManager $em, $entity, array $ch): void ]); } - protected function update(EntityManager $em, $entity, array $ch): void + protected function update(EntityManager $em, object $entity, array $ch): void { $diff = $this->diff($em, $entity, $ch); if (empty($diff)) { @@ -295,7 +320,7 @@ protected function update(EntityManager $em, $entity, array $ch): void ]); } - protected function remove(EntityManager $em, $entity, $id): void + protected function remove(EntityManager $em, object $entity, string|int $id): void { $meta = $em->getClassMetadata(get_class($entity)); $source = array_merge($this->assoc($em, $entity), ['fk' => $id]); @@ -356,7 +381,7 @@ protected function audit(EntityManager $em, array $data): void $this->auditInsertStmt->executeQuery(); } - protected function id(EntityManager $em, $entity) + protected function id(EntityManager $em, object $entity) { $meta = $em->getClassMetadata(get_class($entity)); $pk = $meta->getSingleIdentifierFieldName(); @@ -365,37 +390,68 @@ protected function id(EntityManager $em, $entity) Type::getType($meta->fieldMappings[$pk]['type']), $meta->getReflectionProperty($pk)->getValue($entity) ); + return $pk; } - protected function diff(EntityManager $em, $entity, array $ch): array + protected function isDiff(object $entity, string $fieldName): bool + { + if (array_key_exists($entity::class, $this->entities)) { // New logic + $entityConfig = $this->entities[$entity::class]; + + return ( + ( + $entityConfig['mode'] === 'include' && + ( + !empty($entityConfig['fields']) && in_array($fieldName, $entityConfig['fields']) || + empty($entityConfig['fields']) + ) + ) || + ( + $entityConfig['mode'] === 'exclude' && + !empty($entityConfig['fields']) && !in_array($fieldName, $entityConfig['fields']) + ) + ); + } else { // Old logic + return true; + } + } + + protected function diff(EntityManager $em, object $entity, array $ch): array { $uow = $em->getUnitOfWork(); $meta = $em->getClassMetadata(get_class($entity)); $diff = []; foreach ($ch as $fieldName => list($old, $new)) { if ($meta->hasField($fieldName) && !array_key_exists($fieldName, $meta->embeddedClasses)) { - $mapping = $meta->fieldMappings[$fieldName]; - $diff[$fieldName] = [ - 'old' => $this->value($em, Type::getType($mapping['type']), $old), - 'new' => $this->value($em, Type::getType($mapping['type']), $new), - 'col' => $mapping['columnName'], - ]; + if ($this->isDiff($entity, $fieldName)) { + $mapping = $meta->fieldMappings[$fieldName]; + + $diff[$fieldName] = [ + 'old' => $this->value($em, Type::getType($mapping['type']), $old), + 'new' => $this->value($em, Type::getType($mapping['type']), $new), + 'col' => $mapping['columnName'], + ]; + } } elseif ($meta->hasAssociation($fieldName) && $meta->isSingleValuedAssociation($fieldName)) { - $mapping = $meta->associationMappings[$fieldName]; - $colName = $meta->getSingleAssociationJoinColumnName($fieldName); - $assocMeta = $em->getClassMetadata($mapping['targetEntity']); - $diff[$fieldName] = [ - 'old' => $this->assoc($em, $old), - 'new' => $this->assoc($em, $new), - 'col' => $colName, - ]; + if ($this->isDiff($entity, $fieldName)) { + $mapping = $meta->associationMappings[$fieldName]; + $colName = $meta->getSingleAssociationJoinColumnName($fieldName); + $assocMeta = $em->getClassMetadata($mapping['targetEntity']); + + $diff[$fieldName] = [ + 'old' => $this->assoc($em, $old), + 'new' => $this->assoc($em, $new), + 'col' => $colName, + ]; + } } } + return $diff; } - protected function assoc(EntityManager $em, $association = null): ?array + protected function assoc(EntityManager $em, ?object $association = null): ?array { if (null === $association) { return null; @@ -421,13 +477,14 @@ protected function typ($className): string { // strip prefixes and repeating garbage from name $className = preg_replace("/^(.+\\\)?(.+)(Bundle\\\Entity)/", "$2", $className); + // underscore and lowercase each subdirectory return implode('.', array_map(function ($name) { return strtolower(preg_replace('/(?<=\\w)(?=[A-Z])/', '_$1', $name)); }, explode('\\', $className))); } - protected function label(EntityManager $em, $entity) + protected function label(EntityManager $em, object $entity) { if (is_callable($this->labeler)) { return call_user_func($this->labeler, $entity); @@ -481,10 +538,11 @@ protected function blame(EntityManager $em): ?array if ($token && $token->getUser() instanceof UserInterface && \method_exists($token->getUser(), 'getId')) { return $this->assoc($em, $token->getUser()); } + return null; } - private function getImpersonatorUserFromSecurityToken($token) + private function getImpersonatorUserFromSecurityToken(?TokenInterface $token) { if (false === $this->blameImpersonator) { return null; @@ -498,13 +556,10 @@ private function getImpersonatorUserFromSecurityToken($token) return $role->getSource()->getUser(); } } + return null; } - /** - * @param TokenInterface $token - * @return array - */ private function getRoles(TokenInterface $token): array { if (method_exists($token, 'getRoleNames')) { diff --git a/tests/Entity/Account.php b/tests/Entity/Account.php new file mode 100644 index 0000000..27ad00d --- /dev/null +++ b/tests/Entity/Account.php @@ -0,0 +1,69 @@ +id; + } + + public function setId(int $id): void + { + $this->id = $id; + } + + public function getUsername(): string + { + return $this->username; + } + + public function setUsername(string $username): void + { + $this->username = $username; + } + + public function getPassword(): string + { + return $this->password; + } + + public function setPassword(string $password): void + { + $this->password = $password; + } +} diff --git a/tests/EventListener/AuditListenerExtendsVersionTest.php b/tests/EventListener/AuditListenerExtendsVersionTest.php new file mode 100644 index 0000000..8bdf58a --- /dev/null +++ b/tests/EventListener/AuditListenerExtendsVersionTest.php @@ -0,0 +1,110 @@ +bootKernel([ + 'entities' => [ + Account::class => $dataDogAuditConfig, + ], + ]); + + $this->loadFixtures(); + + $em = $this->getDoctrine()->getManager(); + + $account = new Account(); + $callable($account); + + $em->persist($account); + $em->flush(); + + $auditLogs = $em->createQuery('SELECT l FROM '.AuditLog::class.' l')->getResult(); + $this->assertCount(1, $auditLogs); + + foreach ($expectedFields as $field) { + $this->assertArrayHasKey($field, $auditLogs[0]->getDiff()); + } + } + + public static function dataProvider(): array + { + return [ + 'exclude-password-field' => [ + [ + 'mode' => 'exclude', + 'fields' => [ + 'password', + ], + ], + function (Account $account): void { + $account->setUsername('username'); + $account->setPassword('password'); + }, + [ + 'username', + ], + ], + 'exclude-all' => [ + [ + 'mode' => 'exclude', + 'fields' => null, + ], + function (Account $account): void { + $account->setUsername('username'); + $account->setPassword('password'); + }, + [], + ], + 'include-password-field' => [ + [ + 'mode' => 'include', + 'fields' => [ + 'password', + ], + ], + function (Account $account): void { + $account->setUsername('username'); + $account->setPassword('password'); + }, + [ + 'password', + ], + ], + 'include-all' => [ + [ + 'mode' => 'include', + 'fields' => null, + ], + function (Account $account): void { + $account->setUsername('username'); + $account->setPassword('password'); + }, + [ + 'username', + 'password', + ], + ] + ]; + } +} diff --git a/tests/EventListener/AuditListenerTest.php b/tests/EventListener/AuditListenerTest.php index b084e20..b55c68d 100644 --- a/tests/EventListener/AuditListenerTest.php +++ b/tests/EventListener/AuditListenerTest.php @@ -8,18 +8,13 @@ use DataDog\AuditBundle\Tests\Entity\Post; use DataDog\AuditBundle\Tests\Entity\Tag; use DataDog\AuditBundle\Tests\OrmTestCase; +use DataDog\AuditBundle\Tests\TestKernel; final class AuditListenerTest extends OrmTestCase { - protected function setUp(): void - { - parent::setUp(); - - $this->loadFixtures(); - } - public function testSingleEntityCreation(): void { + $this->includeKernelBoot(); $this->resetDatabase(); $em = $this->getDoctrine()->getManager(); @@ -35,6 +30,7 @@ public function testSingleEntityCreation(): void public function testSingleEntityUpdate(): void { + $this->includeKernelBoot(); $this->resetDatabase(); $em = $this->getDoctrine()->getManager(); @@ -54,6 +50,7 @@ public function testSingleEntityUpdate(): void public function testSingleEntityDelete(): void { + $this->includeKernelBoot(); $this->resetDatabase(); $em = $this->getDoctrine()->getManager(); @@ -73,6 +70,7 @@ public function testSingleEntityDelete(): void public function testEntityRelationCreate(): void { + $this->includeKernelBoot(); $this->resetDatabase(); $em = $this->getDoctrine()->getManager(); @@ -94,6 +92,7 @@ public function testEntityRelationCreate(): void public function testEntityRelationUpdate(): void { + $this->includeKernelBoot(); $this->resetDatabase(); $em = $this->getDoctrine()->getManager(); @@ -120,4 +119,47 @@ public function testEntityRelationUpdate(): void $this->assertCount(6, $em->createQuery('SELECT l FROM '.AuditLog::class.' l')->getResult()); } + + public function testExcludeField(): void + { + $this->excludeKernelBoot(); + $this->resetDatabase(); + + $em = $this->getDoctrine()->getManager(); + + $tag = new Tag(); + $tag->setName('Books'); + + $em->persist($tag); + $em->flush(); + + $tag->setName('Movies'); + + $em->flush(); + + $this->assertCount(2, $em->createQuery('SELECT l FROM '.AuditLog::class.' l')->getResult()); + } + + private function includeKernelBoot(): void + { + $this->bootKernel([ + 'audited_entities' => [ + Tag::class, + Post::class, + ], + ]); + + $this->loadFixtures(); + } + + private function excludeKernelBoot(): void + { + $this->bootKernel([ + 'unaudited_entities' => [ + Tag::class, + ], + ]); + + $this->loadFixtures(); + } } diff --git a/tests/OrmTestCase.php b/tests/OrmTestCase.php index e5c8d58..91e7eba 100644 --- a/tests/OrmTestCase.php +++ b/tests/OrmTestCase.php @@ -14,9 +14,9 @@ abstract class OrmTestCase extends TestCase { protected KernelInterface $kernel; - protected function setUp(): void + protected function bootKernel(array $dataDogAuditConfig): void { - $this->kernel = new TestKernel(); + $this->kernel = new TestKernel($dataDogAuditConfig); $this->kernel->boot(); } diff --git a/tests/TestKernel.php b/tests/TestKernel.php index 518affe..7b36887 100644 --- a/tests/TestKernel.php +++ b/tests/TestKernel.php @@ -17,7 +17,7 @@ class TestKernel extends Kernel { private ?string $projectDir = null; - public function __construct() + public function __construct(private readonly array $dataDogAuditConfig = []) { parent::__construct('test', true); } @@ -55,7 +55,7 @@ public function registerContainerConfiguration(LoaderInterface $loader): void ], ], ]); - $container->loadFromExtension('data_dog_audit'); + $container->loadFromExtension('data_dog_audit', $this->dataDogAuditConfig); $container->register('logger', NullLogger::class); });