diff --git a/CHANGELOG.md b/CHANGELOG.md index 822288e..7d35479 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Admin user-management surface at `/admin/users` per ADR 006. Lists + users scoped by role — `ROLE_ADMIN` sees every user, a + `ROLE_DOMAIN_MANAGER` sees only users whose email domain matches + their own. Optional `?status=pending|approved|blocked` filter for + the approval queue (`/admin/users/pending` redirects to + `?status=pending`). Per-row Approve / Block buttons are gated by + the `ManageUserVoter` from #84 (same-domain check) and the new + `App\Security\UserApproval` service flips the status. The list + view uses a new repository finder + `UserRepository::findVisibleTo()`, scoped against the actor's + role + email domain via the `EmailDomain` helper. CSRF-protected; + `back` parameter on the action forms only honours + `/admin/users…` URLs + ([#64](https://github.com/itk-dev/ai-lib/issues/64), + [#85](https://github.com/itk-dev/ai-lib/issues/85)). - `ROLE_DOMAIN_MANAGER` + `ROLE_ADMIN` role identifiers (`App\Security\Roles`), `role_hierarchy` wiring in `security.yaml` so `ROLE_ADMIN` implies `ROLE_DOMAIN_MANAGER`, and a @@ -19,6 +34,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 queue (#64) and the scoped user-management list view (#85) per ADR 006 ([#84](https://github.com/itk-dev/ai-lib/issues/84)). +- `User.name` (display name) and `User.status` (lifecycle enum: + `pending | approved | blocked`) per ADR 006, plus the + `App\Enum\UserStatus` PHP enum. `UserManager::createUser()` now + requires `name` and accepts an optional `status` (default + `Approved` for the console / fixture path; the registration flow + in #62 will pass `Pending`). The `app:user:create` console command + takes a third `name` argument; fixtures seed Alice + Bob with + display names. Schema is added via a single migration that + backfills any existing rows with `name = ''` and + `status = 'approved'` + ([#45](https://github.com/itk-dev/ai-lib/issues/45), + [#83](https://github.com/itk-dev/ai-lib/issues/83)). - Initial Symfony 8 application scaffold on the ITK Dev Docker `symfony-8` template (phpfpm 8.4, nginx, MariaDB, Mailpit, Traefik), including dev dependencies for coding standards (`php-cs-fixer`, diff --git a/config/packages/security.yaml b/config/packages/security.yaml index 9b0a2da..46c8ecd 100644 --- a/config/packages/security.yaml +++ b/config/packages/security.yaml @@ -37,6 +37,7 @@ security: access_control: - { path: ^/login, roles: PUBLIC_ACCESS } - { path: ^/logout, roles: PUBLIC_ACCESS } + - { path: ^/admin, roles: ROLE_DOMAIN_MANAGER } when@test: security: diff --git a/migrations/Version20260619080000.php b/migrations/Version20260619080000.php new file mode 100644 index 0000000..1c1fa37 --- /dev/null +++ b/migrations/Version20260619080000.php @@ -0,0 +1,41 @@ +getTable('user'); + $user->addColumn('name', Types::STRING, ['length' => 255, 'notnull' => true, 'default' => '']); + $user->addColumn('status', Types::STRING, ['length' => 32, 'notnull' => true, 'default' => 'approved']); + } + + public function down(Schema $schema): void + { + $user = $schema->getTable('user'); + $user->dropColumn('status'); + $user->dropColumn('name'); + } +} diff --git a/src/Command/UserCreateCommand.php b/src/Command/UserCreateCommand.php index c9d642b..e1c7591 100644 --- a/src/Command/UserCreateCommand.php +++ b/src/Command/UserCreateCommand.php @@ -38,6 +38,7 @@ protected function configure(): void { $this ->addArgument('email', InputArgument::REQUIRED, 'The user\'s e-mail address (must be unique).') + ->addArgument('name', InputArgument::REQUIRED, 'The user\'s display name.') ->addArgument('password', InputArgument::REQUIRED, 'The user\'s password in clear-text — will be hashed.'); } @@ -53,10 +54,11 @@ protected function execute(InputInterface $input, OutputInterface $output): int { $io = new SymfonyStyle($input, $output); $email = (string) $input->getArgument('email'); + $name = (string) $input->getArgument('name'); $password = (string) $input->getArgument('password'); try { - $user = $this->userManager->createUser($email, $password); + $user = $this->userManager->createUser($email, $name, $password); } catch (\DomainException|\InvalidArgumentException $e) { $io->error($e->getMessage()); diff --git a/src/Controller/Admin/UserController.php b/src/Controller/Admin/UserController.php new file mode 100644 index 0000000..2cbe820 --- /dev/null +++ b/src/Controller/Admin/UserController.php @@ -0,0 +1,100 @@ +currentUser(); + $statusFilter = $this->resolveStatusFilter((string) $request->query->get('status', '')); + + return $this->render('admin/user/list.html.twig', [ + 'users' => $this->userRepository->findVisibleTo($actor, $statusFilter), + 'status_filter' => $statusFilter, + ]); + } + + #[Route(path: '/admin/users/pending', name: 'app_admin_users_pending', methods: ['GET'])] + public function pending(): Response + { + return $this->redirectToRoute('app_admin_users', ['status' => UserStatus::Pending->value]); + } + + #[Route(path: '/admin/users/{id}/approve', name: 'app_admin_user_approve', methods: ['POST'], requirements: ['id' => '\d+'])] + #[IsGranted(ManageUserVoter::APPROVE, subject: 'user')] + public function approve(User $user, Request $request): Response + { + if (!$this->isCsrfTokenValid('admin-user-action', (string) $request->request->get('_token'))) { + throw $this->createAccessDeniedException(); + } + + $this->userApproval->approve($user); + $this->addFlash('success', 'admin.users.flash.approved'); + + return $this->redirectToBackUrl($request); + } + + #[Route(path: '/admin/users/{id}/block', name: 'app_admin_user_block', methods: ['POST'], requirements: ['id' => '\d+'])] + #[IsGranted(ManageUserVoter::BLOCK, subject: 'user')] + public function block(User $user, Request $request): Response + { + if (!$this->isCsrfTokenValid('admin-user-action', (string) $request->request->get('_token'))) { + throw $this->createAccessDeniedException(); + } + + $this->userApproval->block($user); + $this->addFlash('success', 'admin.users.flash.blocked'); + + return $this->redirectToBackUrl($request); + } + + private function currentUser(): User + { + $user = $this->getUser(); + \assert($user instanceof User); + + return $user; + } + + private function resolveStatusFilter(string $raw): ?UserStatus + { + if ('' === $raw) { + return null; + } + + return UserStatus::tryFrom($raw); + } + + private function redirectToBackUrl(Request $request): Response + { + $back = (string) $request->request->get('back', ''); + if (str_starts_with($back, '/admin/users')) { + return $this->redirect($back); + } + + return $this->redirectToRoute('app_admin_users'); + } +} diff --git a/src/DataFixtures/UserFixtures.php b/src/DataFixtures/UserFixtures.php index 67e4dd7..c713ed1 100644 --- a/src/DataFixtures/UserFixtures.php +++ b/src/DataFixtures/UserFixtures.php @@ -31,7 +31,7 @@ public function __construct(private readonly UserManager $userManager) */ public function load(ObjectManager $manager): void { - $this->userManager->createUser('alice@example.test', 'password'); - $this->userManager->createUser('bob@example.test', 'password'); + $this->userManager->createUser('alice@example.test', 'Alice', 'password'); + $this->userManager->createUser('bob@example.test', 'Bob', 'password'); } } diff --git a/src/Entity/User.php b/src/Entity/User.php index 8fee99a..6315ef2 100644 --- a/src/Entity/User.php +++ b/src/Entity/User.php @@ -2,7 +2,9 @@ namespace App\Entity; +use App\Enum\UserStatus; use App\Repository\UserRepository; +use Doctrine\DBAL\Types\Types; use Doctrine\ORM\Mapping as ORM; use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface; use Symfony\Component\Security\Core\User\UserInterface; @@ -32,6 +34,12 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface #[ORM\Column] private ?string $password = null; + #[ORM\Column(length: 255)] + private string $name = ''; + + #[ORM\Column(type: Types::STRING, length: 32, enumType: UserStatus::class)] + private UserStatus $status = UserStatus::Pending; + public function getId(): ?int { return $this->id; @@ -96,6 +104,30 @@ public function setPassword(string $password): static return $this; } + public function getName(): string + { + return $this->name; + } + + public function setName(string $name): static + { + $this->name = $name; + + return $this; + } + + public function getStatus(): UserStatus + { + return $this->status; + } + + public function setStatus(UserStatus $status): static + { + $this->status = $status; + + return $this; + } + /** * Ensure the session doesn't contain actual password hashes by CRC32C-hashing them, as supported since Symfony 7.3. */ diff --git a/src/Enum/UserStatus.php b/src/Enum/UserStatus.php new file mode 100644 index 0000000..ad96874 --- /dev/null +++ b/src/Enum/UserStatus.php @@ -0,0 +1,19 @@ +getEntityManager()->persist($user); $this->getEntityManager()->flush(); } + + /** + * Find users visible to the acting user, optionally filtered by status. + * + * Decision flow mirrors {@see \App\Security\Voter\ManageUserVoter}: + * + * 1. Site admins (`ROLE_ADMIN`) see every user. + * 2. Domain managers (`ROLE_DOMAIN_MANAGER` without admin) see only + * users whose email domain matches their own. + * 3. Everyone else gets an empty result — the caller must still + * gate the route via `IsGranted` first, this is a belt-and- + * braces filter for query-level scoping. + * + * @param User $actor the acting user (whose role + email determine the scope) + * @param UserStatus|null $statusFilter optional status filter for #64's approval-queue view + * + * @return list users sorted by id ascending + */ + public function findVisibleTo(User $actor, ?UserStatus $statusFilter = null): array + { + $qb = $this->createQueryBuilder('u') + ->orderBy('u.id', 'ASC'); + + $roles = $actor->getRoles(); + + if (\in_array(Roles::ADMIN, $roles, true)) { + // Admin sees everyone — no domain filter. + } elseif (\in_array(Roles::DOMAIN_MANAGER, $roles, true)) { + $domain = EmailDomain::of($actor); + if (null === $domain) { + return []; + } + $qb->andWhere('LOWER(u.email) LIKE :domainSuffix') + ->setParameter('domainSuffix', '%@'.$domain); + } else { + return []; + } + + if (null !== $statusFilter) { + $qb->andWhere('u.status = :status') + ->setParameter('status', $statusFilter->value); + } + + /** @var list $result */ + $result = $qb->getQuery()->getResult(); + + return $result; + } } diff --git a/src/Security/UserApproval.php b/src/Security/UserApproval.php new file mode 100644 index 0000000..07c7cf5 --- /dev/null +++ b/src/Security/UserApproval.php @@ -0,0 +1,61 @@ +setStatus(UserStatus::Approved); + $this->entityManager->flush(); + } + + /** + * Mark the user as blocked so they cannot sign in (but their row + * is preserved for audit and possible un-blocking). + * + * Safe to call when the user is already `Blocked` — the flush + * becomes a no-op. + * + * @param User $user the user to block + */ + public function block(User $user): void + { + $user->setStatus(UserStatus::Blocked); + $this->entityManager->flush(); + } +} diff --git a/src/Security/UserManager.php b/src/Security/UserManager.php index 4ddac9f..97ef954 100644 --- a/src/Security/UserManager.php +++ b/src/Security/UserManager.php @@ -5,6 +5,7 @@ namespace App\Security; use App\Entity\User; +use App\Enum\UserStatus; use App\Repository\UserRepository; use Doctrine\ORM\EntityManagerInterface; use Symfony\Component\PasswordHasher\Hasher\UserPasswordHasherInterface; @@ -33,17 +34,29 @@ public function __construct( /** * Create a new persisted user with a hashed password. * + * Defaults to `UserStatus::Approved` so the console / fixture paths + * land a usable account immediately. The registration flow (#62) + * passes `UserStatus::Pending` explicitly so a domain manager has + * to approve the user before they can sign in. + * * @param string $email user e-mail; must be unique + * @param string $name display name; required, may be any non-null string * @param string $plainPassword clear-text password, hashed before persistence * @param list $roles additional roles beyond the implicit `ROLE_USER` + * @param UserStatus $status identity-lifecycle status; defaults to {@see UserStatus::Approved} * * @return User the persisted user with an assigned id * * @throws \DomainException when a user with the same e-mail already exists * @throws \InvalidArgumentException when `$plainPassword` is empty */ - public function createUser(string $email, string $plainPassword, array $roles = []): User - { + public function createUser( + string $email, + string $name, + string $plainPassword, + array $roles = [], + UserStatus $status = UserStatus::Approved, + ): User { if ('' === $plainPassword) { throw new \InvalidArgumentException('Password must not be empty.'); } @@ -54,7 +67,9 @@ public function createUser(string $email, string $plainPassword, array $roles = $user = (new User()) ->setEmail($email) - ->setRoles($roles); + ->setName($name) + ->setRoles($roles) + ->setStatus($status); $user->setPassword($this->passwordHasher->hashPassword($user, $plainPassword)); $this->entityManager->persist($user); diff --git a/templates/admin/user/list.html.twig b/templates/admin/user/list.html.twig new file mode 100644 index 0000000..a6b5f9c --- /dev/null +++ b/templates/admin/user/list.html.twig @@ -0,0 +1,80 @@ +{% extends 'base.html.twig' %} + +{% block title %}{{ 'admin.users.title'|trans({'%brand%': brand_name}) }}{% endblock %} + +{% block body %} +
+ {{ 'admin.users.eyebrow'|trans }} +

+ {{ 'admin.users.heading'|trans }} +

+ + {% for flash in app.flashes('success') %} +
+ {{ flash|trans }} +
+ {% endfor %} + + + + {% if users is empty %} +

{{ 'admin.users.empty'|trans }}

+ {% else %} +
    + {% for u in users %} +
  • +
    +
    +

    {{ u.name }}

    +

    {{ u.email }}

    +
    + + {{ ('admin.users.status.' ~ u.status.value)|trans }} + +
    + +
    + {% set back_url = path('app_admin_users', status_filter ? {status: status_filter.value} : {}) %} + {% if u.status.value != 'approved' %} +
    + + + + {{ 'admin.users.action.approve'|trans }} + +
    + {% endif %} + + {% if u.status.value != 'blocked' %} +
    + + + + {{ 'admin.users.action.block'|trans }} + +
    + {% endif %} +
    +
    • + {% endfor %} +
+ {% endif %} +
+{% endblock %} diff --git a/tests/Integration/Command/UserCreateCommandTest.php b/tests/Integration/Command/UserCreateCommandTest.php index 880756b..39c27f7 100644 --- a/tests/Integration/Command/UserCreateCommandTest.php +++ b/tests/Integration/Command/UserCreateCommandTest.php @@ -30,6 +30,7 @@ public function testCreatesUser(): void { $exit = $this->tester->execute([ 'email' => 'charlie@example.test', + 'name' => 'Charlie', 'password' => 'secret', ]); @@ -42,6 +43,7 @@ public function testReportsFailureWhenEmailAlreadyExists(): void // alice@example.test is in the baseline fixtures. $exit = $this->tester->execute([ 'email' => 'alice@example.test', + 'name' => 'Alice', 'password' => 'second', ]); diff --git a/tests/Integration/Controller/Admin/UserControllerTest.php b/tests/Integration/Controller/Admin/UserControllerTest.php new file mode 100644 index 0000000..cd3c20f --- /dev/null +++ b/tests/Integration/Controller/Admin/UserControllerTest.php @@ -0,0 +1,244 @@ +client = self::createClient(); + } + + public function testAnonymousIsRedirectedToLogin(): void + { + $this->client->request('GET', '/admin/users'); + + self::assertResponseRedirects(); + self::assertStringContainsString('/login', (string) $this->client->getResponse()->headers->get('Location')); + } + + public function testPlainUserGets403(): void + { + $this->loginAsApproved('alice@example.test'); + + $this->client->request('GET', '/admin/users'); + + self::assertResponseStatusCodeSame(403); + } + + public function testAdminSeesEveryUser(): void + { + $um = self::getContainer()->get(UserManager::class); + $um->createUser('admin@example.test', 'Admin', 'pw', [Roles::ADMIN]); + $um->createUser('eve@other.test', 'Eve', 'pw'); + + $this->loginAsApproved('admin@example.test'); + + $crawler = $this->client->request('GET', '/admin/users'); + + self::assertResponseIsSuccessful(); + $body = $crawler->filter('body')->text(); + self::assertStringContainsString('alice@example.test', $body); + self::assertStringContainsString('eve@other.test', $body); + } + + public function testDomainManagerSeesOnlySameDomainUsers(): void + { + $um = self::getContainer()->get(UserManager::class); + $um->createUser('dm@example.test', 'DM', 'pw', [Roles::DOMAIN_MANAGER]); + $um->createUser('outsider@other.test', 'Outsider', 'pw'); + + $this->loginAsApproved('dm@example.test'); + + $crawler = $this->client->request('GET', '/admin/users'); + + self::assertResponseIsSuccessful(); + $body = $crawler->filter('body')->text(); + self::assertStringContainsString('alice@example.test', $body); + self::assertStringNotContainsString('outsider@other.test', $body); + } + + public function testPendingSubrouteRedirectsToFilteredList(): void + { + $um = self::getContainer()->get(UserManager::class); + $um->createUser('admin@example.test', 'Admin', 'pw', [Roles::ADMIN]); + $this->loginAsApproved('admin@example.test'); + + $this->client->request('GET', '/admin/users/pending'); + + self::assertResponseRedirects('/admin/users?status=pending'); + } + + public function testStatusFilterRendersOnlyMatchingRows(): void + { + $um = self::getContainer()->get(UserManager::class); + $um->createUser('admin@example.test', 'Admin', 'pw', [Roles::ADMIN]); + $um->createUser('pending@example.test', 'Pending User', 'pw', status: UserStatus::Pending); + $this->loginAsApproved('admin@example.test'); + + $crawler = $this->client->request('GET', '/admin/users?status=pending'); + + self::assertResponseIsSuccessful(); + $body = $crawler->filter('body')->text(); + self::assertStringContainsString('pending@example.test', $body); + // Approved baseline alice should not appear in a `pending` filter. + self::assertStringNotContainsString('alice@example.test', $body); + } + + public function testInvalidStatusFilterIsTreatedAsNoFilter(): void + { + $um = self::getContainer()->get(UserManager::class); + $um->createUser('admin@example.test', 'Admin', 'pw', [Roles::ADMIN]); + $this->loginAsApproved('admin@example.test'); + + $crawler = $this->client->request('GET', '/admin/users?status=garbage'); + + self::assertResponseIsSuccessful(); + // alice (approved) is rendered regardless because the filter falls back to null. + self::assertStringContainsString('alice@example.test', $crawler->filter('body')->text()); + } + + public function testApproveActionFlipsStatusToApproved(): void + { + $um = self::getContainer()->get(UserManager::class); + $um->createUser('admin@example.test', 'Admin', 'pw', [Roles::ADMIN]); + $target = $um->createUser('target@example.test', 'Target', 'pw', status: UserStatus::Pending); + + $this->loginAsApproved('admin@example.test'); + + $crawler = $this->client->request('GET', '/admin/users?status=pending'); + $form = $crawler->filter('form[action$="/approve"]')->form(); + $this->client->submit($form); + + self::assertResponseRedirects('/admin/users?status=pending'); + + $reloaded = self::getContainer()->get(UserRepository::class)->find($target->getId()); + self::assertNotNull($reloaded); + self::assertSame(UserStatus::Approved, $reloaded->getStatus()); + } + + public function testBlockActionFlipsStatusToBlocked(): void + { + $um = self::getContainer()->get(UserManager::class); + $um->createUser('admin@example.test', 'Admin', 'pw', [Roles::ADMIN]); + $target = $um->createUser('target@example.test', 'Target', 'pw'); + + $this->loginAsApproved('admin@example.test'); + + $crawler = $this->client->request('GET', '/admin/users'); + $form = $crawler->filter('form[action$="/'.$target->getId().'/block"]')->form(); + $this->client->submit($form); + + self::assertResponseRedirects('/admin/users'); + + $reloaded = self::getContainer()->get(UserRepository::class)->find($target->getId()); + self::assertNotNull($reloaded); + self::assertSame(UserStatus::Blocked, $reloaded->getStatus()); + } + + public function testApproveActionRejectsInvalidCsrfToken(): void + { + $um = self::getContainer()->get(UserManager::class); + $um->createUser('admin@example.test', 'Admin', 'pw', [Roles::ADMIN]); + $target = $um->createUser('target@example.test', 'Target', 'pw', status: UserStatus::Pending); + + $this->loginAsApproved('admin@example.test'); + + $this->client->request('POST', '/admin/users/'.$target->getId().'/approve', [ + '_token' => 'nope', + ]); + + self::assertResponseStatusCodeSame(403); + + $reloaded = self::getContainer()->get(UserRepository::class)->find($target->getId()); + self::assertNotNull($reloaded); + self::assertSame(UserStatus::Pending, $reloaded->getStatus(), 'CSRF rejection must not have flipped the status.'); + } + + public function testBlockActionRejectsInvalidCsrfToken(): void + { + $um = self::getContainer()->get(UserManager::class); + $um->createUser('admin@example.test', 'Admin', 'pw', [Roles::ADMIN]); + $target = $um->createUser('target@example.test', 'Target', 'pw'); + + $this->loginAsApproved('admin@example.test'); + + $this->client->request('POST', '/admin/users/'.$target->getId().'/block', [ + '_token' => 'nope', + ]); + + self::assertResponseStatusCodeSame(403); + + $reloaded = self::getContainer()->get(UserRepository::class)->find($target->getId()); + self::assertNotNull($reloaded); + self::assertSame(UserStatus::Approved, $reloaded->getStatus(), 'CSRF rejection must not have flipped the status.'); + } + + public function testApproveActionDeniedAcrossDomainsForDomainManager(): void + { + $um = self::getContainer()->get(UserManager::class); + $um->createUser('dm@example.test', 'DM', 'pw', [Roles::DOMAIN_MANAGER]); + $target = $um->createUser('foreign@other.test', 'Foreign', 'pw', status: UserStatus::Pending); + + $this->loginAsApproved('dm@example.test'); + + // Direct POST against a cross-domain target — the voter on the + // `IsGranted` attribute fails closed before the controller body runs, + // regardless of the CSRF token shape. + $this->client->request('POST', '/admin/users/'.$target->getId().'/approve', [ + '_token' => 'irrelevant', + ]); + + self::assertSame(403, $this->client->getResponse()->getStatusCode()); + + $reloaded = self::getContainer()->get(UserRepository::class)->find($target->getId()); + self::assertNotNull($reloaded); + self::assertSame(UserStatus::Pending, $reloaded->getStatus()); + } + + public function testBackParameterRespectsTheAdminScope(): void + { + $um = self::getContainer()->get(UserManager::class); + $um->createUser('admin@example.test', 'Admin', 'pw', [Roles::ADMIN]); + $um->createUser('victim@example.test', 'Victim', 'pw'); + $this->loginAsApproved('admin@example.test'); + + // Crafted POST with an off-site `back` parameter — controller must ignore it. + $crawler = $this->client->request('GET', '/admin/users'); + /** @var User $target */ + $target = self::getContainer()->get(UserRepository::class)->findOneBy(['email' => 'victim@example.test']); + $token = $crawler->filter('input[name="_token"]')->first()->attr('value'); + + $this->client->request('POST', '/admin/users/'.$target->getId().'/block', [ + '_token' => $token, + 'back' => 'https://evil.invalid/owned', + ]); + + self::assertResponseRedirects('/admin/users'); + } + + private function loginAsApproved(string $email): void + { + $user = self::getContainer()->get(UserRepository::class)->findOneBy(['email' => $email]); + \assert(null !== $user, 'Test user must be created before login.'); + $this->client->loginUser($user); + } +} diff --git a/tests/Integration/Repository/UserRepositoryTest.php b/tests/Integration/Repository/UserRepositoryTest.php index 239fa3d..367ccc0 100644 --- a/tests/Integration/Repository/UserRepositoryTest.php +++ b/tests/Integration/Repository/UserRepositoryTest.php @@ -4,7 +4,12 @@ namespace App\Tests\Integration\Repository; +use App\Entity\User; +use App\Enum\UserStatus; use App\Repository\UserRepository; +use App\Security\Roles; +use App\Security\UserManager; +use Doctrine\ORM\EntityManagerInterface; use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; use Symfony\Component\Security\Core\Exception\UnsupportedUserException; use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface; @@ -62,4 +67,83 @@ public function getPassword(): ?string $this->repository->upgradePassword($foreignUser, 'irrelevant'); } + + public function testStatusEnumRoundTripsThroughThePersistedRow(): void + { + $em = self::getContainer()->get(EntityManagerInterface::class); + + $user = (new User()) + ->setEmail('eve@example.test') + ->setName('Eve') + ->setPassword('hash') + ->setStatus(UserStatus::Blocked); + $em->persist($user); + $em->flush(); + $em->clear(); + + $reloaded = $this->repository->find($user->getId()); + + self::assertNotNull($reloaded); + self::assertSame('Eve', $reloaded->getName()); + self::assertSame(UserStatus::Blocked, $reloaded->getStatus()); + } + + public function testFindVisibleToReturnsEveryUserForAdmin(): void + { + $manager = self::getContainer()->get(UserManager::class); + $admin = $manager->createUser('admin@example.test', 'Admin', 'pw', [Roles::ADMIN]); + $manager->createUser('eve@other.test', 'Eve', 'pw', status: UserStatus::Pending); + + $emails = array_map( + static fn (User $u): ?string => $u->getEmail(), + $this->repository->findVisibleTo($admin), + ); + + self::assertContains('alice@example.test', $emails); + self::assertContains('bob@example.test', $emails); + self::assertContains('admin@example.test', $emails); + self::assertContains('eve@other.test', $emails); + } + + public function testFindVisibleToScopesByDomainForDomainManager(): void + { + $manager = self::getContainer()->get(UserManager::class); + $domainManager = $manager->createUser('dm@example.test', 'DM', 'pw', [Roles::DOMAIN_MANAGER]); + $manager->createUser('outsider@other.test', 'Outsider', 'pw'); + + $emails = array_map( + static fn (User $u): ?string => $u->getEmail(), + $this->repository->findVisibleTo($domainManager), + ); + + self::assertContains('alice@example.test', $emails); + self::assertContains('bob@example.test', $emails); + self::assertContains('dm@example.test', $emails); + self::assertNotContains('outsider@other.test', $emails); + + // A plain authenticated user (no DOMAIN_MANAGER / ADMIN role) sees + // no one — the repository falls through to an empty result. + $alice = $this->repository->findOneBy(['email' => 'alice@example.test']); + self::assertNotNull($alice); + self::assertSame([], $this->repository->findVisibleTo($alice)); + + // Defensive: a domain manager with no email also gets an empty + // result rather than running a query against an unresolved domain. + $headless = (new User())->setRoles([Roles::DOMAIN_MANAGER]); + self::assertSame([], $this->repository->findVisibleTo($headless)); + } + + public function testFindVisibleToFiltersByStatus(): void + { + $manager = self::getContainer()->get(UserManager::class); + $admin = $manager->createUser('siteadmin@example.test', 'Site Admin', 'pw', [Roles::ADMIN]); + $manager->createUser('pending@example.test', 'Pending', 'pw', status: UserStatus::Pending); + + $emails = array_map( + static fn (User $u): ?string => $u->getEmail(), + $this->repository->findVisibleTo($admin, UserStatus::Pending), + ); + + self::assertSame(['pending@example.test'], $emails); + } } diff --git a/tests/Integration/Security/UserApprovalTest.php b/tests/Integration/Security/UserApprovalTest.php new file mode 100644 index 0000000..38f5c19 --- /dev/null +++ b/tests/Integration/Security/UserApprovalTest.php @@ -0,0 +1,73 @@ +userManager = $container->get(UserManager::class); + $this->userApproval = $container->get(UserApproval::class); + } + + public function testApproveTransitionsPendingUserToApproved(): void + { + $user = $this->userManager->createUser( + 'kim@example.test', + 'Kim', + 'pw', + status: UserStatus::Pending, + ); + + $this->userApproval->approve($user); + + self::assertSame(UserStatus::Approved, $user->getStatus()); + } + + public function testBlockTransitionsApprovedUserToBlocked(): void + { + $user = $this->userManager->createUser( + 'lara@example.test', + 'Lara', + 'pw', + ); + + $this->userApproval->block($user); + + self::assertSame(UserStatus::Blocked, $user->getStatus()); + } + + public function testApprovalIsRoundTrippedThroughTheDatabase(): void + { + $user = $this->userManager->createUser( + 'mona@example.test', + 'Mona', + 'pw', + status: UserStatus::Pending, + ); + $id = $user->getId(); + + $this->userApproval->approve($user); + + self::bootKernel(); + $reloaded = self::getContainer() + ->get(\App\Repository\UserRepository::class) + ->find($id); + + self::assertNotNull($reloaded); + self::assertSame(UserStatus::Approved, $reloaded->getStatus()); + } +} diff --git a/tests/Integration/Security/UserManagerTest.php b/tests/Integration/Security/UserManagerTest.php index 56b150a..87eeecb 100644 --- a/tests/Integration/Security/UserManagerTest.php +++ b/tests/Integration/Security/UserManagerTest.php @@ -4,6 +4,7 @@ namespace App\Tests\Integration\Security; +use App\Enum\UserStatus; use App\Repository\UserRepository; use App\Security\UserManager; use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; @@ -35,10 +36,12 @@ protected function setUp(): void public function testCreatesAndPersistsUserWithHashedPassword(): void { - $user = $this->userManager->createUser('charlie@example.test', 'secret'); + $user = $this->userManager->createUser('charlie@example.test', 'Charlie', 'secret'); self::assertNotNull($user->getId()); self::assertSame('charlie@example.test', $user->getEmail()); + self::assertSame('Charlie', $user->getName()); + self::assertSame(UserStatus::Approved, $user->getStatus()); self::assertSame(['ROLE_USER'], $user->getRoles()); self::assertNotSame('secret', $user->getPassword(), 'Password must be hashed.'); self::assertTrue( @@ -50,18 +53,30 @@ public function testCreatesAndPersistsUserWithHashedPassword(): void public function testCreateUserStoresExtraRoles(): void { - $user = $this->userManager->createUser('admin@example.test', 'secret', ['ROLE_ADMIN']); + $user = $this->userManager->createUser('admin@example.test', 'Admin', 'secret', ['ROLE_ADMIN']); self::assertSame(['ROLE_ADMIN', 'ROLE_USER'], $user->getRoles()); } + public function testCreateUserHonoursExplicitStatus(): void + { + $user = $this->userManager->createUser( + 'dora@example.test', + 'Dora', + 'secret', + status: UserStatus::Pending, + ); + + self::assertSame(UserStatus::Pending, $user->getStatus()); + } + public function testCreateUserRejectsDuplicateEmail(): void { // alice@example.test is loaded by UserFixtures in the bootstrap. $this->expectException(\DomainException::class); $this->expectExceptionMessage('alice@example.test'); - $this->userManager->createUser('alice@example.test', 'other'); + $this->userManager->createUser('alice@example.test', 'Alice', 'other'); } public function testCreateUserRejectsEmptyPassword(): void @@ -69,7 +84,7 @@ public function testCreateUserRejectsEmptyPassword(): void $this->expectException(\InvalidArgumentException::class); $this->expectExceptionMessage('Password must not be empty.'); - $this->userManager->createUser('charlie@example.test', ''); + $this->userManager->createUser('charlie@example.test', 'Charlie', ''); } public function testChangePasswordReplacesTheHash(): void diff --git a/tests/Unit/DataFixtures/UserFixturesTest.php b/tests/Unit/DataFixtures/UserFixturesTest.php index 7a6dd7e..1ff2465 100644 --- a/tests/Unit/DataFixtures/UserFixturesTest.php +++ b/tests/Unit/DataFixtures/UserFixturesTest.php @@ -6,6 +6,7 @@ use App\DataFixtures\UserFixtures; use App\Entity\User; +use App\Enum\UserStatus; use App\Repository\UserRepository; use App\Security\UserManager; use Doctrine\ORM\EntityManagerInterface; @@ -30,12 +31,12 @@ public function testLoadPersistsAliceAndBobWithFixturePassword(): void $userRepository->method('findOneBy')->willReturn(null); $passwordHasher->method('hashPassword')->willReturn('hashed'); - $persistedEmails = []; + $persisted = []; $entityManager->expects(self::exactly(2)) ->method('persist') - ->willReturnCallback(function (object $entity) use (&$persistedEmails): void { + ->willReturnCallback(function (object $entity) use (&$persisted): void { \assert($entity instanceof User); - $persistedEmails[] = $entity->getEmail(); + $persisted[] = $entity; }); $entityManager->expects(self::exactly(2))->method('flush'); @@ -44,6 +45,12 @@ public function testLoadPersistsAliceAndBobWithFixturePassword(): void $fixture->load($this->createMock(ObjectManager::class)); - self::assertSame(['alice@example.test', 'bob@example.test'], $persistedEmails); + $emails = array_map(static fn (User $u): ?string => $u->getEmail(), $persisted); + $names = array_map(static fn (User $u): string => $u->getName(), $persisted); + $statuses = array_map(static fn (User $u): UserStatus => $u->getStatus(), $persisted); + + self::assertSame(['alice@example.test', 'bob@example.test'], $emails); + self::assertSame(['Alice', 'Bob'], $names); + self::assertSame([UserStatus::Approved, UserStatus::Approved], $statuses); } } diff --git a/tests/Unit/Entity/UserTest.php b/tests/Unit/Entity/UserTest.php index c549749..0eef22f 100644 --- a/tests/Unit/Entity/UserTest.php +++ b/tests/Unit/Entity/UserTest.php @@ -5,10 +5,20 @@ namespace App\Tests\Unit\Entity; use App\Entity\User; +use App\Enum\UserStatus; use PHPUnit\Framework\TestCase; final class UserTest extends TestCase { + public function testConstructorDefaultsStatusToPending(): void + { + $user = new User(); + + self::assertSame(UserStatus::Pending, $user->getStatus()); + self::assertSame('', $user->getName()); + } + + public function testGetRolesAlwaysIncludesRoleUser(): void { $user = new User(); @@ -69,6 +79,12 @@ public function testSettersMutateAndReturnStatic(): void self::assertSame($user, $user->setRoles(['ROLE_EDITOR'])); self::assertSame(['ROLE_EDITOR', 'ROLE_USER'], $user->getRoles()); + self::assertSame($user, $user->setName('Bob')); + self::assertSame('Bob', $user->getName()); + + self::assertSame($user, $user->setStatus(UserStatus::Approved)); + self::assertSame(UserStatus::Approved, $user->getStatus()); + self::assertNull($user->getId()); } } diff --git a/translations/messages.da.yaml b/translations/messages.da.yaml index f71b193..ae55193 100644 --- a/translations/messages.da.yaml +++ b/translations/messages.da.yaml @@ -42,6 +42,29 @@ security: password_label: "Adgangskode" submit: "Log ind" +admin: + users: + title: "Brugere – %brand%" + eyebrow: "Administration" + heading: "Brugere" + empty: "Ingen brugere matcher det valgte filter." + filter_label: "Filtrer brugere efter status" + filter: + all: "Alle" + pending: "Afventer godkendelse" + approved: "Godkendte" + blocked: "Spærrede" + status: + pending: "Afventer" + approved: "Godkendt" + blocked: "Spærret" + action: + approve: "Godkend" + block: "Spær" + flash: + approved: "Brugeren er nu godkendt og kan logge ind." + blocked: "Brugeren er spærret og kan ikke logge ind." + assistant: detail: title: "%title% – %brand%"