fix: hold an account to the password lifetime policy when it is edited

src/Application/Account/Ports/AccountPresetService.php

@@ -46,6 +46,22 @@ interface AccountPresetService
      */
     public function checkPasswordPreset(AccountDto $accountDto): AccountDto;
 
+    /**
+     * Holds an account to the policy's password lifetime, without asking about the password.
+     *
+     * For the paths that change an account without setting a password: they write
+     * `passDateChange` from a field the form offers, and the cap a fixed preset sets is a maximum
+     * that has to survive an edit. Separate from `checkPasswordPreset()` because that validates
+     * the password too, and an edit legitimately carries none.
+     *
+     * @template T of AccountDto
+     * @param T $accountDto
+     * @return T
+     * @throws ConstraintException
+     * @throws QueryException
+     */
+    public function checkPasswordExpiry(AccountDto $accountDto): AccountDto;
+
     /**
      * @throws QueryException
      * @throws ConstraintException

src/Application/Account/Services/AccountPreset.php

@@ -79,25 +79,75 @@ public function checkPasswordPreset(AccountDto $accountDto): AccountDto
             if ($passwordPreset !== null) {
                 $this->passwordValidator->validate($passwordPreset, $accountDto->pass);
 
-                if ($this->configData->isAccountExpireEnabled()) {
-                    $expireTimePreset = $passwordPreset->getExpireTime();
-
-                    if ($expireTimePreset > 0) {
-                        $maxPassDateChange = time() + $expireTimePreset;
-
-                        if (empty($accountDto->passDateChange)
-                            || $accountDto->passDateChange > $maxPassDateChange
-                        ) {
-                            return $accountDto->withPassDateChange($maxPassDateChange);
-                        }
-                    }
-                }
+                return $this->clampToPolicyLifetime($accountDto, $passwordPreset);
             }
         }
 
         return $accountDto;
     }
 
+    /**
+     * Holds an account to the policy's password lifetime, without asking about the password.
+     *
+     * The lifetime a fixed preset sets is a maximum, and it used to be applied only where a
+     * password was being set — creating an account, copying one, changing its password. Editing
+     * the account writes `passDateChange` just the same, from a field the form offers, and none of
+     * those paths clamped it: an account created under a ninety-day policy could be edited a
+     * moment later to expire in a decade. Bulk edit could do it to a selection at once.
+     *
+     * Separate from `checkPasswordPreset()` because that also validates the password against the
+     * preset, and an edit legitimately carries none — `PasswordValidator::validate()` measures
+     * `mb_strlen('')` against the required length and throws, so calling the whole check here
+     * would refuse every edit while a fixed preset existed.
+     *
+     * @template T of AccountDto
+     * @param T $accountDto
+     * @return T
+     * @throws ConstraintException
+     * @throws QueryException
+     * @throws SPException
+     */
+    public function checkPasswordExpiry(AccountDto $accountDto): AccountDto
+    {
+        $itemPreset = $this->itemPresetService->getForCurrentUser(ItemPresetInterface::ITEM_TYPE_ACCOUNT_PASSWORD);
+
+        if ($itemPreset === null || $itemPreset->getFixed() !== 1) {
+            return $accountDto;
+        }
+
+        $passwordPreset = $itemPreset->hydrate(Password::class);
+
+        return $passwordPreset === null
+            ? $accountDto
+            : $this->clampToPolicyLifetime($accountDto, $passwordPreset);
+    }
+
+    /**
+     * @template T of AccountDto
+     * @param T $accountDto
+     * @return T
+     */
+    private function clampToPolicyLifetime(AccountDto $accountDto, Password $passwordPreset): AccountDto
+    {
+        if (!$this->configData->isAccountExpireEnabled()) {
+            return $accountDto;
+        }
+
+        $expireTimePreset = $passwordPreset->getExpireTime();
+
+        if ($expireTimePreset <= 0) {
+            return $accountDto;
+        }
+
+        $maxPassDateChange = time() + $expireTimePreset;
+
+        if (empty($accountDto->passDateChange) || $accountDto->passDateChange > $maxPassDateChange) {
+            return $accountDto->withPassDateChange($maxPassDateChange);
+        }
+
+        return $accountDto;
+    }
+
     /**
      * @param int $accountId
      * @throws ConstraintException

src/Infrastructure/Adapter/In/Api/Controllers/Account/EditController.php

@@ -59,6 +59,11 @@ public function editAction(): ApiResponse
             )
         );
 
+        // The expiry only, not the whole preset check: an edit carries no password, and
+        // validating one against the policy would refuse every edit while a fixed preset existed.
+        // The lifetime a fixed preset sets is a maximum, and `expireDate` is a parameter here.
+        $accountUpdateDto = $this->accountPresetService->checkPasswordExpiry($accountUpdateDto);
+
         $this->accountService->update($accountUpdateDto->id, $accountUpdateDto);
 
         $accountDetails = $this->accountService->getByIdEnriched($accountUpdateDto->id);

src/Infrastructure/Adapter/In/Web/Forms/AccountForm.php

@@ -82,6 +82,10 @@ public function validateFor(int $action, ?int $id = null): FormInterface
             AclActionsInterface::ACCOUNT_EDIT =>
             $chain->next(fn(AccountDto $dto) => $this->analyzeItems($dto))
                   ->next(fn(AccountDto $dto) => $this->checkCommon($dto))
+                  // The expiry only, not the whole preset check: an edit carries no password, and
+                  // validating one against the policy would refuse every edit. The lifetime a
+                  // fixed preset sets is a maximum, and the form offers the field.
+                  ->next(fn(AccountDto $dto) => $this->accountPresetService->checkPasswordExpiry($dto))
                   ->resolve(),
             AclActionsInterface::ACCOUNT_CREATE,
             AclActionsInterface::ACCOUNT_COPY =>
@@ -97,6 +101,8 @@ public function validateFor(int $action, ?int $id = null): FormInterface
             AclActionsInterface::ACCOUNTMGR_BULK_EDIT =>
             $chain->next(fn(AccountDto $dto) => $this->analyzeItems($dto))
                   ->next(fn(AccountDto $dto) => $this->analyzeBulkEdit($dto))
+                  // Bulk edit writes passDateChange for every account in the selection.
+                  ->next(fn(AccountDto $dto) => $this->accountPresetService->checkPasswordExpiry($dto))
                   ->resolve(),
             // Guard the public FormInterface contract: an unexpected action must fail as a
             // handled validation error, not an \UnhandledMatchError (a 500 for the client).

tests/Unit/Application/Account/Services/AccountPresetTest.php

@@ -223,6 +223,114 @@ public function testCheckPasswordPresetWithLaterPassDateChangeIsClampedToPolicyL
         self::assertLessThanOrEqual($after + $expireTimePreset, $out->passDateChange);
     }
 
+    /**
+     * The lifetime cap survives an edit, and asks nothing about the password.
+     *
+     * The clamp used to live only in `checkPasswordPreset()`, which is called where a password is
+     * being set — creating an account, copying one, changing its password. Editing an account
+     * writes `passDateChange` just the same, from a field the form offers, and none of those paths
+     * clamped it: an account created under a ninety-day policy could be edited a moment later to
+     * expire in a decade, and bulk edit could do it to a selection at once.
+     *
+     * The password validator must not run here. An edit carries no password, and
+     * `PasswordValidator::validate()` measures `mb_strlen('')` against the required length and
+     * throws — so validating would refuse every edit while a fixed preset existed. `expects(never())`
+     * is the point of this test as much as the clamp is.
+     *
+     * @throws ConstraintException
+     * @throws QueryException
+     * @throws SPException
+     */
+    public function testCheckPasswordExpiryClampsAnEditWithoutValidatingThePassword(): void
+    {
+        $expireDays = self::$faker->numberBetween(1, 30);
+        $expireTimePreset = $expireDays * Password::EXPIRE_TIME_MULTIPLIER;
+
+        $itemPreset = ItemPresetDataGenerator::factory()
+            ->buildItemPresetData($this->buildPasswordPresetWithExpireDays($expireDays))
+            ->mutate(['fixed' => 1]);
+
+        $this->itemPresetService
+            ->expects(self::once())
+            ->method('getForCurrentUser')
+            ->with(ItemPresetInterface::ITEM_TYPE_ACCOUNT_PASSWORD)
+            ->willReturn($itemPreset);
+
+        $this->passwordValidator
+            ->expects(self::never())
+            ->method('validate');
+
+        // An edit: no password, and a deadline further out than the policy allows.
+        $accountDto = AccountDataGenerator::factory()->buildAccountUpdateDto()
+                                          ->mutate([
+                                              'pass' => '',
+                                              'passDateChange' => time() + $expireTimePreset + 31536000,
+                                          ]);
+
+        $before = time();
+        $out = $this->accountPreset->checkPasswordExpiry($accountDto);
+        $after = time();
+
+        self::assertGreaterThanOrEqual($before + $expireTimePreset, $out->passDateChange);
+        self::assertLessThanOrEqual($after + $expireTimePreset, $out->passDateChange);
+    }
+
+    /**
+     * A deadline already stricter than the policy is left alone by the edit path too.
+     *
+     * @throws ConstraintException
+     * @throws QueryException
+     * @throws SPException
+     */
+    public function testCheckPasswordExpiryLeavesAStricterDeadlineAlone(): void
+    {
+        $expireDays = self::$faker->numberBetween(10, 30);
+
+        $itemPreset = ItemPresetDataGenerator::factory()
+            ->buildItemPresetData($this->buildPasswordPresetWithExpireDays($expireDays))
+            ->mutate(['fixed' => 1]);
+
+        $this->itemPresetService
+            ->expects(self::once())
+            ->method('getForCurrentUser')
+            ->willReturn($itemPreset);
+
+        $stricter = time() + 60;
+
+        $out = $this->accountPreset->checkPasswordExpiry(
+            AccountDataGenerator::factory()->buildAccountUpdateDto()->mutate(['passDateChange' => $stricter])
+        );
+
+        self::assertSame($stricter, $out->passDateChange, 'the cap is a ceiling, not a floor');
+    }
+
+    /**
+     * A preset that is not fixed is a suggestion, and an edit is left as it was.
+     *
+     * @throws ConstraintException
+     * @throws QueryException
+     * @throws SPException
+     */
+    public function testCheckPasswordExpiryIgnoresAPresetThatIsNotFixed(): void
+    {
+        $itemPreset = ItemPresetDataGenerator::factory()
+            ->buildItemPresetData($this->buildPasswordPresetWithExpireDays(1))
+            ->mutate(['fixed' => 0]);
+
+        $this->itemPresetService
+            ->expects(self::once())
+            ->method('getForCurrentUser')
+            ->willReturn($itemPreset);
+
+        $wanted = time() + 31536000;
+
+        $out = $this->accountPreset->checkPasswordExpiry(
+            AccountDataGenerator::factory()->buildAccountUpdateDto()->mutate(['passDateChange' => $wanted])
+        );
+
+        self::assertSame($wanted, $out->passDateChange);
+    }
+
     /**
      * A "fixed" preset's expiry acts as a CEILING, not a floor: a deadline that is
      * already earlier (stricter) than the policy's limit must be left untouched.

tests/Unit/Infrastructure/Adapter/In/Web/Forms/AccountFormTest.php

@@ -76,6 +76,7 @@ public function testPrivateFlagsReachTheDto(): void
 
         $accountPresetService = $this->createMock(AccountPresetService::class);
         $accountPresetService->method('checkPasswordPreset')->willReturnArgument(0);
+        $accountPresetService->method('checkPasswordExpiry')->willReturnArgument(0);
 
         $form = new AccountForm($this->application, $request, $accountPresetService);
 
@@ -115,6 +116,7 @@ public function testPrivateFlagsAreRefusedWithoutThePermission(): void
 
         $accountPresetService = $this->createMock(AccountPresetService::class);
         $accountPresetService->method('checkPasswordPreset')->willReturnArgument(0);
+        $accountPresetService->method('checkPasswordExpiry')->willReturnArgument(0);
 
         $form = new AccountForm($this->application, $request, $accountPresetService);
 
@@ -207,6 +209,7 @@ public function testChildAccountSkipsPasswordChecks(): void
     {
         $accountPresetService = $this->createStub(AccountPresetService::class);
         $accountPresetService->method('checkPasswordPreset')->willReturnArgument(0);
+        $accountPresetService->method('checkPasswordExpiry')->willReturnArgument(0);
 
         $form = new AccountForm(
             $this->application,
@@ -229,6 +232,7 @@ public function testEditPassSucceedsWithMatchingPasswords(): void
     {
         $accountPresetService = $this->createStub(AccountPresetService::class);
         $accountPresetService->method('checkPasswordPreset')->willReturnArgument(0);
+        $accountPresetService->method('checkPasswordExpiry')->willReturnArgument(0);
 
         $form = new AccountForm(
             $this->application,
@@ -326,6 +330,7 @@ private function buildForm(RequestService $request): AccountForm
     {
         $accountPresetService = $this->createStub(AccountPresetService::class);
         $accountPresetService->method('checkPasswordPreset')->willReturnArgument(0);
+        $accountPresetService->method('checkPasswordExpiry')->willReturnArgument(0);
 
         return new AccountForm($this->application, $request, $accountPresetService);
     }