feat(security): reject pending and blocked users at login (#63)

CHANGELOG.md

@@ -9,6 +9,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
+- `App\Security\AccountStatusChecker` implementing
+  `UserCheckerInterface` — gates the login flow so a `User` with
+  `status = Pending` or `status = Blocked` is rejected before the
+  password is verified. Throws
+  `CustomUserMessageAccountStatusException` with the localised
+  translation keys `account.pending` and `account.blocked` (rendered
+  in the `security` domain — see `translations/security.da.yaml`).
+  Wired on the `main` firewall via `security.yaml`'s `user_checker:`
+  key
+  ([#63](https://github.com/itk-dev/ai-lib/issues/63)).
 - `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

config/packages/security.yaml

@@ -19,6 +19,7 @@ security:
         main:
             lazy: true
             provider: app_user_provider
+            user_checker: App\Security\AccountStatusChecker
             form_login:
                 login_path: app_login
                 check_path: app_login

src/Security/AccountStatusChecker.php

@@ -0,0 +1,65 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Security;
+
+use App\Entity\User;
+use App\Enum\UserStatus;
+use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
+use Symfony\Component\Security\Core\Exception\CustomUserMessageAccountStatusException;
+use Symfony\Component\Security\Core\User\UserCheckerInterface;
+use Symfony\Component\Security\Core\User\UserInterface;
+
+/**
+ * Reject login attempts for any {@see User} whose {@see UserStatus} is
+ * not {@see UserStatus::Approved}.
+ *
+ * Wired on the `main` firewall via `security.yaml`'s `user_checker:`
+ * key. Symfony Security calls {@see self::checkPreAuth()} before the
+ * password is verified; throwing a
+ * {@see CustomUserMessageAccountStatusException} halts the flow and
+ * surfaces the (localised) translation key on the login form.
+ *
+ * A `Blocked` user retains the roles
+ * they had before being blocked — they just can't sign in to exercise
+ * them.
+ */
+final class AccountStatusChecker implements UserCheckerInterface
+{
+    /**
+     * Refuse pending and blocked users before the password is checked.
+     *
+     * Non-`User` implementations fall through (the password checker
+     * will reject them on its own terms).
+     *
+     * @param UserInterface       $user  the user attempting to authenticate
+     * @param TokenInterface|null $token unused; Symfony 8 added the slot for hooks that need it
+     *
+     * @throws CustomUserMessageAccountStatusException when status is Pending or Blocked
+     */
+    public function checkPreAuth(UserInterface $user, ?TokenInterface $token = null): void
+    {
+        if (!$user instanceof User) {
+            return;
+        }
+
+        // Keys live in the `security` translation domain — see
+        // translations/security.da.yaml.
+        match ($user->getStatus()) {
+            UserStatus::Pending => throw new CustomUserMessageAccountStatusException('account.pending'),
+            UserStatus::Blocked => throw new CustomUserMessageAccountStatusException('account.blocked'),
+            UserStatus::Approved => null,
+        };
+    }
+
+    /**
+     * Post-auth hook required by the interface; no checks needed here.
+     *
+     * @param UserInterface       $user  the user that just authenticated successfully
+     * @param TokenInterface|null $token unused; Symfony 8 added the slot for hooks that need it
+     */
+    public function checkPostAuth(UserInterface $user, ?TokenInterface $token = null): void
+    {
+    }
+}

tests/Integration/Controller/SecurityControllerTest.php

@@ -6,6 +6,8 @@
 
 use App\Controller\SecurityController;
 use App\Entity\User;
+use App\Enum\UserStatus;
+use App\Security\UserManager;
 use Symfony\Bundle\FrameworkBundle\KernelBrowser;
 use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
 
@@ -100,4 +102,63 @@ public function testLogoutClearsTheSession(): void
             $this->client->getContainer()->get('security.token_storage')->getToken(),
         );
     }
+
+    // Tests that a Pending user is rejected at login with the localised pending message.
+    public function testPendingUserCannotLogIn(): void
+    {
+        $this->client->getContainer()->get(UserManager::class)->createUser(
+            'carol@example.test',
+            'Carol',
+            'password',
+            status: UserStatus::Pending,
+        );
+
+        $crawler = $this->client->request('GET', '/login');
+        $form = $crawler->filter('form')->form();
+        $form['_username'] = 'carol@example.test';
+        $form['_password'] = 'password';
+        $this->client->submit($form);
+
+        self::assertResponseRedirects('/login');
+        $crawler = $this->client->followRedirect();
+        self::assertResponseIsSuccessful();
+
+        // Localised pending message is rendered on the form.
+        self::assertStringContainsString(
+            'venter på godkendelse',
+            $crawler->filter('body')->text(),
+        );
+        self::assertNull(
+            $this->client->getContainer()->get('security.token_storage')->getToken(),
+        );
+    }
+
+    // Tests that a Blocked user is rejected at login with the localised blocked message.
+    public function testBlockedUserCannotLogIn(): void
+    {
+        $this->client->getContainer()->get(UserManager::class)->createUser(
+            'dora@example.test',
+            'Dora',
+            'password',
+            status: UserStatus::Blocked,
+        );
+
+        $crawler = $this->client->request('GET', '/login');
+        $form = $crawler->filter('form')->form();
+        $form['_username'] = 'dora@example.test';
+        $form['_password'] = 'password';
+        $this->client->submit($form);
+
+        self::assertResponseRedirects('/login');
+        $crawler = $this->client->followRedirect();
+        self::assertResponseIsSuccessful();
+
+        self::assertStringContainsString(
+            'spærret',
+            $crawler->filter('body')->text(),
+        );
+        self::assertNull(
+            $this->client->getContainer()->get('security.token_storage')->getToken(),
+        );
+    }
 }

tests/Unit/Security/AccountStatusCheckerTest.php

@@ -0,0 +1,74 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Unit\Security;
+
+use App\Entity\User;
+use App\Enum\UserStatus;
+use App\Security\AccountStatusChecker;
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\Security\Core\Exception\CustomUserMessageAccountStatusException;
+use Symfony\Component\Security\Core\User\UserInterface;
+
+final class AccountStatusCheckerTest extends TestCase
+{
+    // Tests that an Approved user passes the pre-auth hook without raising.
+    public function testApprovedUserPassesPreAuth(): void
+    {
+        $user = (new User())
+            ->setName('Alice')
+            ->setStatus(UserStatus::Approved);
+
+        (new AccountStatusChecker())->checkPreAuth($user);
+
+        // No exception thrown is the assertion; explicit to keep PHPUnit happy.
+        self::assertTrue(true);
+    }
+
+    // Ensures a Pending user is rejected with the 'account.pending' message key.
+    public function testPendingUserIsRejectedWithLocalisedMessage(): void
+    {
+        $user = (new User())
+            ->setName('Pending')
+            ->setStatus(UserStatus::Pending);
+
+        $this->expectException(CustomUserMessageAccountStatusException::class);
+        $this->expectExceptionMessage('account.pending');
+
+        (new AccountStatusChecker())->checkPreAuth($user);
+    }
+
+    // Ensures a Blocked user is rejected with the 'account.blocked' message key.
+    public function testBlockedUserIsRejectedWithLocalisedMessage(): void
+    {
+        $user = (new User())
+            ->setName('Blocked')
+            ->setStatus(UserStatus::Blocked);
+
+        $this->expectException(CustomUserMessageAccountStatusException::class);
+        $this->expectExceptionMessage('account.blocked');
+
+        (new AccountStatusChecker())->checkPreAuth($user);
+    }
+
+    // Verifies non-App User implementations fall through to the password checker.
+    public function testForeignUserImplementationsAreIgnored(): void
+    {
+        $foreignUser = $this->createMock(UserInterface::class);
+
+        (new AccountStatusChecker())->checkPreAuth($foreignUser);
+
+        self::assertTrue(true);
+    }
+
+    // Tests that checkPostAuth does nothing (required by the interface).
+    public function testCheckPostAuthIsANoOp(): void
+    {
+        $user = (new User())->setStatus(UserStatus::Approved);
+
+        (new AccountStatusChecker())->checkPostAuth($user);
+
+        self::assertTrue(true);
+    }
+}

translations/security.da.yaml

@@ -0,0 +1,10 @@
+# Danish strings rendered in the `security` translation domain.
+#
+# Symfony Security's CustomUserMessageAccountStatusException carries a
+# `messageKey` that the login template renders as
+# `error.messageKey|trans(error.messageData, 'security')`. The keys
+# below match the messageKeys thrown by App\Security\AccountStatusChecker.
+
+account:
+    pending: "Din konto venter på godkendelse fra en administrator."
+    blocked: "Din konto er spærret. Kontakt en administrator for at få den åbnet igen."