test(user) : couvre le soft-delete + désarchivage admin et corrige les retours de review
- ajoute des tests fonctionnels (archive au DELETE, exclusion de la
collection, listing/désarchivage admin, anti-auto-archivage) et un test
unitaire du ArchivedUserChecker
- expose un filtre BooleanFilter `archived` + bypass admin dans
ExcludeArchivedUserExtension pour lister les archivés (?archived=true)
- rend `archived` modifiable par un admin (groupe user:write + ApiProperty
ROLE_ADMIN) → désarchivage possible via PATCH /api/users/{id}
- RestoreMissingUsersCommand : ne compte que les insertions réelles
(ON CONFLICT DO NOTHING n'est plus comptabilisé à tort)
- relève memory_limit des tests à 512M (boot sérialiseur API Platform)
This commit is contained in:
@@ -13,6 +13,10 @@
|
|||||||
<php>
|
<php>
|
||||||
<ini name="display_errors" value="1" />
|
<ini name="display_errors" value="1" />
|
||||||
<ini name="error_reporting" value="-1" />
|
<ini name="error_reporting" value="-1" />
|
||||||
|
<!-- API Platform's serializer/metadata boot is memory-hungry on the first
|
||||||
|
call in a process (cold phpdoc + serializer metadata). 128M is too
|
||||||
|
tight for non-paginated collections such as GET /api/users. -->
|
||||||
|
<ini name="memory_limit" value="512M" />
|
||||||
<server name="APP_ENV" value="test" force="true" />
|
<server name="APP_ENV" value="test" force="true" />
|
||||||
<server name="SHELL_VERBOSITY" value="-1" />
|
<server name="SHELL_VERBOSITY" value="-1" />
|
||||||
<server name="KERNEL_CLASS" value="App\Kernel" />
|
<server name="KERNEL_CLASS" value="App\Kernel" />
|
||||||
|
|||||||
@@ -102,7 +102,7 @@ final class RestoreMissingUsersCommand extends Command
|
|||||||
$created = 0;
|
$created = 0;
|
||||||
|
|
||||||
foreach ($missingIds as $id) {
|
foreach ($missingIds as $id) {
|
||||||
$this->connection->executeStatement(
|
$inserted = $this->connection->executeStatement(
|
||||||
<<<'SQL'
|
<<<'SQL'
|
||||||
INSERT INTO "user"
|
INSERT INTO "user"
|
||||||
(id, username, first_name, last_name, roles, password, created_at,
|
(id, username, first_name, last_name, roles, password, created_at,
|
||||||
@@ -122,8 +122,14 @@ final class RestoreMissingUsersCommand extends Command
|
|||||||
'password' => $hash,
|
'password' => $hash,
|
||||||
],
|
],
|
||||||
);
|
);
|
||||||
++$created;
|
|
||||||
$io->writeln(sprintf(' ✓ user #%d recreated (archived)', $id));
|
// ON CONFLICT may have skipped an already-present row — only count real inserts.
|
||||||
|
if ($inserted > 0) {
|
||||||
|
++$created;
|
||||||
|
$io->writeln(sprintf(' ✓ user #%d recreated (archived)', $id));
|
||||||
|
} else {
|
||||||
|
$io->writeln(sprintf(' • user #%d already present — skipped', $id));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
$io->success(sprintf('%d user(s) restored as archived. References are valid again — no data lost.', $created));
|
$io->success(sprintf('%d user(s) restored as archived. References are valid again — no data lost.', $created));
|
||||||
|
|||||||
@@ -4,6 +4,8 @@ declare(strict_types=1);
|
|||||||
|
|
||||||
namespace App\Module\Core\Domain\Entity;
|
namespace App\Module\Core\Domain\Entity;
|
||||||
|
|
||||||
|
use ApiPlatform\Doctrine\Orm\Filter\BooleanFilter;
|
||||||
|
use ApiPlatform\Metadata\ApiFilter;
|
||||||
use ApiPlatform\Metadata\ApiProperty;
|
use ApiPlatform\Metadata\ApiProperty;
|
||||||
use ApiPlatform\Metadata\ApiResource;
|
use ApiPlatform\Metadata\ApiResource;
|
||||||
use ApiPlatform\Metadata\Delete;
|
use ApiPlatform\Metadata\Delete;
|
||||||
@@ -64,6 +66,9 @@ use Symfony\Component\Serializer\Attribute\Groups;
|
|||||||
],
|
],
|
||||||
denormalizationContext: ['groups' => ['user:write']],
|
denormalizationContext: ['groups' => ['user:write']],
|
||||||
)]
|
)]
|
||||||
|
// Archived users are hidden from the default /users collection by
|
||||||
|
// ExcludeArchivedUserExtension; an admin can still list them with ?archived=true.
|
||||||
|
#[ApiFilter(BooleanFilter::class, properties: ['archived'])]
|
||||||
#[Auditable]
|
#[Auditable]
|
||||||
#[ORM\Entity(repositoryClass: DoctrineUserRepository::class)]
|
#[ORM\Entity(repositoryClass: DoctrineUserRepository::class)]
|
||||||
#[ORM\Table(name: '`user`')]
|
#[ORM\Table(name: '`user`')]
|
||||||
@@ -118,7 +123,8 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface, SharedU
|
|||||||
* from selectable user lists.
|
* from selectable user lists.
|
||||||
*/
|
*/
|
||||||
#[ORM\Column(options: ['default' => false])]
|
#[ORM\Column(options: ['default' => false])]
|
||||||
#[Groups(['me:read', 'user:list'])]
|
#[ApiProperty(security: "is_granted('ROLE_ADMIN')")]
|
||||||
|
#[Groups(['me:read', 'user:list', 'user:write'])]
|
||||||
private bool $archived = false;
|
private bool $archived = false;
|
||||||
|
|
||||||
// --- HR / absence management fields (readable only by an admin or the user themselves) ---
|
// --- HR / absence management fields (readable only by an admin or the user themselves) ---
|
||||||
|
|||||||
+16
-1
@@ -9,14 +9,23 @@ use ApiPlatform\Doctrine\Orm\Util\QueryNameGeneratorInterface;
|
|||||||
use ApiPlatform\Metadata\Operation;
|
use ApiPlatform\Metadata\Operation;
|
||||||
use App\Module\Core\Domain\Entity\User;
|
use App\Module\Core\Domain\Entity\User;
|
||||||
use Doctrine\ORM\QueryBuilder;
|
use Doctrine\ORM\QueryBuilder;
|
||||||
|
use Symfony\Bundle\SecurityBundle\Security;
|
||||||
|
|
||||||
|
use function array_key_exists;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Hides archived (soft-deleted) users from the `/users` collection so they are
|
* Hides archived (soft-deleted) users from the `/users` collection so they are
|
||||||
* no longer offered as assignees/collaborators, while existing references to
|
* no longer offered as assignees/collaborators, while existing references to
|
||||||
* them (already stored on tasks, time entries…) keep resolving normally.
|
* them (already stored on tasks, time entries…) keep resolving normally.
|
||||||
|
*
|
||||||
|
* An admin can opt back in to see archived users — e.g. to restore one — by
|
||||||
|
* passing the `archived` query filter explicitly (`?archived=true`), in which
|
||||||
|
* case the BooleanFilter declared on User handles the predicate instead.
|
||||||
*/
|
*/
|
||||||
final class ExcludeArchivedUserExtension implements QueryCollectionExtensionInterface
|
final readonly class ExcludeArchivedUserExtension implements QueryCollectionExtensionInterface
|
||||||
{
|
{
|
||||||
|
public function __construct(private Security $security) {}
|
||||||
|
|
||||||
public function applyToCollection(
|
public function applyToCollection(
|
||||||
QueryBuilder $queryBuilder,
|
QueryBuilder $queryBuilder,
|
||||||
QueryNameGeneratorInterface $queryNameGenerator,
|
QueryNameGeneratorInterface $queryNameGenerator,
|
||||||
@@ -28,6 +37,12 @@ final class ExcludeArchivedUserExtension implements QueryCollectionExtensionInte
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Let an admin explicitly query archived users via ?archived=...
|
||||||
|
$filters = $context['filters'] ?? [];
|
||||||
|
if (array_key_exists('archived', $filters) && $this->security->isGranted('ROLE_ADMIN')) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
$alias = $queryBuilder->getRootAliases()[0];
|
$alias = $queryBuilder->getRootAliases()[0];
|
||||||
$queryBuilder->andWhere(sprintf('%s.archived = false', $alias));
|
$queryBuilder->andWhere(sprintf('%s.archived = false', $alias));
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,145 @@
|
|||||||
|
<?php
|
||||||
|
|
||||||
|
declare(strict_types=1);
|
||||||
|
|
||||||
|
namespace App\Tests\Functional\Module\Core;
|
||||||
|
|
||||||
|
use App\Module\Core\Domain\Entity\User;
|
||||||
|
use Doctrine\ORM\EntityManagerInterface;
|
||||||
|
use Symfony\Bundle\FrameworkBundle\KernelBrowser;
|
||||||
|
use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Covers the soft-delete behaviour: deleting a user archives it (the row is
|
||||||
|
* kept so referencing tasks/time entries still serialize), archived users are
|
||||||
|
* hidden from the default collection but an admin can list and restore them.
|
||||||
|
*
|
||||||
|
* @internal
|
||||||
|
*/
|
||||||
|
final class UserArchiveApiTest extends WebTestCase
|
||||||
|
{
|
||||||
|
public function testDeleteArchivesUserInsteadOfRemovingIt(): void
|
||||||
|
{
|
||||||
|
$client = self::createClient();
|
||||||
|
$em = self::getContainer()->get(EntityManagerInterface::class);
|
||||||
|
$target = $this->createUser($em, 'archive-target-'.uniqid());
|
||||||
|
$em->flush();
|
||||||
|
$targetId = $target->getId();
|
||||||
|
|
||||||
|
$this->loginAdmin($client);
|
||||||
|
$client->request('DELETE', '/api/users/'.$targetId);
|
||||||
|
|
||||||
|
self::assertResponseStatusCodeSame(204);
|
||||||
|
|
||||||
|
$em->clear();
|
||||||
|
$reloaded = $em->getRepository(User::class)->find($targetId);
|
||||||
|
self::assertInstanceOf(User::class, $reloaded, 'Row must still exist (soft delete)');
|
||||||
|
self::assertTrue($reloaded->isArchived(), 'User must be flagged archived');
|
||||||
|
self::assertNull($reloaded->getApiToken(), 'API token must be cleared on archive');
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testAdminCannotArchiveOwnAccount(): void
|
||||||
|
{
|
||||||
|
$client = self::createClient();
|
||||||
|
$em = self::getContainer()->get(EntityManagerInterface::class);
|
||||||
|
$this->loginAdmin($client);
|
||||||
|
$adminId = $this->userId('admin');
|
||||||
|
|
||||||
|
$client->request('DELETE', '/api/users/'.$adminId);
|
||||||
|
|
||||||
|
self::assertResponseStatusCodeSame(403);
|
||||||
|
|
||||||
|
$em->clear();
|
||||||
|
$admin = $em->getRepository(User::class)->find($adminId);
|
||||||
|
self::assertFalse($admin->isArchived(), 'Admin must not have archived itself');
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testArchivedUserIsHiddenFromDefaultCollection(): void
|
||||||
|
{
|
||||||
|
$client = self::createClient();
|
||||||
|
$username = $this->createArchivedUser();
|
||||||
|
|
||||||
|
$this->loginAdmin($client);
|
||||||
|
$client->request('GET', '/api/users', server: ['HTTP_ACCEPT' => 'application/json']);
|
||||||
|
|
||||||
|
self::assertResponseIsSuccessful();
|
||||||
|
$usernames = array_column(json_decode($client->getResponse()->getContent(), true), 'username');
|
||||||
|
self::assertNotContains($username, $usernames, 'Archived user must not appear in the default list');
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testAdminCanListArchivedUsersViaFilter(): void
|
||||||
|
{
|
||||||
|
$client = self::createClient();
|
||||||
|
$username = $this->createArchivedUser();
|
||||||
|
|
||||||
|
$this->loginAdmin($client);
|
||||||
|
$client->request('GET', '/api/users?archived=true', server: ['HTTP_ACCEPT' => 'application/json']);
|
||||||
|
|
||||||
|
self::assertResponseIsSuccessful();
|
||||||
|
$usernames = array_column(json_decode($client->getResponse()->getContent(), true), 'username');
|
||||||
|
self::assertContains($username, $usernames, 'Admin must be able to list archived users via ?archived=true');
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testAdminCanRestoreUserViaPatch(): void
|
||||||
|
{
|
||||||
|
$client = self::createClient();
|
||||||
|
$em = self::getContainer()->get(EntityManagerInterface::class);
|
||||||
|
|
||||||
|
$user = $this->createUser($em, 'restore-target-'.uniqid());
|
||||||
|
$user->setArchived(true);
|
||||||
|
$em->flush();
|
||||||
|
$userId = $user->getId();
|
||||||
|
$em->clear();
|
||||||
|
|
||||||
|
$this->loginAdmin($client);
|
||||||
|
$client->request('PATCH', '/api/users/'.$userId, server: [
|
||||||
|
'CONTENT_TYPE' => 'application/merge-patch+json',
|
||||||
|
], content: json_encode(['archived' => false]));
|
||||||
|
|
||||||
|
self::assertResponseIsSuccessful();
|
||||||
|
|
||||||
|
$em->clear();
|
||||||
|
$reloaded = $em->getRepository(User::class)->find($userId);
|
||||||
|
self::assertFalse($reloaded->isArchived(), 'Admin PATCH must be able to un-archive a user');
|
||||||
|
}
|
||||||
|
|
||||||
|
private function createArchivedUser(): string
|
||||||
|
{
|
||||||
|
$em = self::getContainer()->get(EntityManagerInterface::class);
|
||||||
|
$username = 'archived-'.uniqid();
|
||||||
|
$user = $this->createUser($em, $username);
|
||||||
|
$user->setArchived(true);
|
||||||
|
$em->flush();
|
||||||
|
$em->clear();
|
||||||
|
|
||||||
|
return $username;
|
||||||
|
}
|
||||||
|
|
||||||
|
private function createUser(EntityManagerInterface $em, string $username): User
|
||||||
|
{
|
||||||
|
$user = new User();
|
||||||
|
$user->setUsername($username);
|
||||||
|
$user->setPassword('x');
|
||||||
|
$user->setRoles(['ROLE_USER']);
|
||||||
|
$em->persist($user);
|
||||||
|
|
||||||
|
return $user;
|
||||||
|
}
|
||||||
|
|
||||||
|
private function loginAdmin(KernelBrowser $client): void
|
||||||
|
{
|
||||||
|
$em = self::getContainer()->get(EntityManagerInterface::class);
|
||||||
|
$user = $em->getRepository(User::class)->findOneBy(['username' => 'admin']);
|
||||||
|
self::assertInstanceOf(User::class, $user);
|
||||||
|
$client->loginUser($user);
|
||||||
|
}
|
||||||
|
|
||||||
|
private function userId(string $username): int
|
||||||
|
{
|
||||||
|
$em = self::getContainer()->get(EntityManagerInterface::class);
|
||||||
|
$user = $em->getRepository(User::class)->findOneBy(['username' => $username]);
|
||||||
|
self::assertInstanceOf(User::class, $user);
|
||||||
|
|
||||||
|
return $user->getId();
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,43 @@
|
|||||||
|
<?php
|
||||||
|
|
||||||
|
declare(strict_types=1);
|
||||||
|
|
||||||
|
namespace App\Tests\Unit\Module\Core\Infrastructure\Security;
|
||||||
|
|
||||||
|
use App\Module\Core\Domain\Entity\User;
|
||||||
|
use App\Module\Core\Infrastructure\Security\ArchivedUserChecker;
|
||||||
|
use PHPUnit\Framework\TestCase;
|
||||||
|
use Symfony\Component\Security\Core\Exception\CustomUserMessageAccountStatusException;
|
||||||
|
use Symfony\Component\Security\Core\User\InMemoryUser;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @internal
|
||||||
|
*/
|
||||||
|
final class ArchivedUserCheckerTest extends TestCase
|
||||||
|
{
|
||||||
|
public function testArchivedUserIsRejectedPreAuth(): void
|
||||||
|
{
|
||||||
|
$user = new User()->setArchived(true);
|
||||||
|
|
||||||
|
$this->expectException(CustomUserMessageAccountStatusException::class);
|
||||||
|
|
||||||
|
new ArchivedUserChecker()->checkPreAuth($user);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testActiveUserPassesPreAuth(): void
|
||||||
|
{
|
||||||
|
$user = new User()->setArchived(false);
|
||||||
|
|
||||||
|
new ArchivedUserChecker()->checkPreAuth($user);
|
||||||
|
|
||||||
|
$this->addToAssertionCount(1);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testNonAppUserIsIgnored(): void
|
||||||
|
{
|
||||||
|
// A user that is not our entity must not be rejected by this checker.
|
||||||
|
new ArchivedUserChecker()->checkPreAuth(new InMemoryUser('someone', null));
|
||||||
|
|
||||||
|
$this->addToAssertionCount(1);
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user