fix: a master password is held to its minimum at every door

CLAUDE.md

@@ -476,6 +476,8 @@ wrote `private` / `privateGroup` as sent; it is now `Account::privacyAllowedFor(
 the account will actually have.
 The account manager's grid left out an account private to somebody else, while its delete and
 bulk edit acted on whatever ids were posted; `Account::assertNotWithheldAsPrivate()` guards those.
+The master password's eleven-character minimum was enforced only by the installer (and in bytes);
+`MasterPass::assertLongEnough()` now holds the web settings, their hash-only option and the CLI to it.
 It runs the other way too, and that is the more useful half: the API re-reads the user on every
 request and refuses a disabled one, while the web trusted what login had put in the session — so
 disabling an account stopped its token at once and left its browser session working, and since the

src/Application/Crypt/Services/MasterPass.php

@@ -35,6 +35,8 @@
 use SP\Domain\Common\Services\ServiceException;
 use SP\Application\Config\Ports\ConfigService;
 use SP\Domain\Core\Exceptions\ConstraintException;
+use SP\Domain\Core\Exceptions\InvalidArgumentException;
+use SP\Domain\Core\Exceptions\SPException;
 use SP\Domain\Core\Exceptions\QueryException;
 use SP\Domain\Crypt\Dtos\UpdateMasterPassRequest;
 use SP\Application\Crypt\Ports\MasterPassService;
@@ -52,6 +54,11 @@ final class MasterPass extends Service implements MasterPassService
     public const PARAM_MASTER_PASS_TIME = 'lastupdatempass';
     public const PARAM_MASTER_PASS_HASH = 'masterPwd';
 
+    /**
+     * The shortest master password accepted, in characters
+     */
+    public const MIN_LENGTH = 11;
+
     public function __construct(
         Application                                   $application,
         private readonly ConfigService                $configService,
@@ -93,6 +100,29 @@ public function checkMasterPassword(string $masterPassword): bool
         return false;
     }
 
+    /**
+     * Refuse a master password shorter than MIN_LENGTH characters.
+     *
+     * Every account secret is sealed with the master password, so its minimum is the floor under
+     * the whole vault. The installer enforced it and nothing else did: the web's encryption
+     * settings (both the full rotation and the "hash only" option) and `sp:updateMasterPassword`
+     * accepted any non-empty value, so an installation could be re-keyed to `a` the day after it
+     * was set up. The installer also measured it with strlen(), in bytes, so four CJK characters
+     * (twelve bytes) passed a rule that promises eleven characters.
+     *
+     * @throws InvalidArgumentException
+     */
+    public static function assertLongEnough(?string $masterPassword): void
+    {
+        if (mb_strlen($masterPassword ?? '') < self::MIN_LENGTH) {
+            throw new InvalidArgumentException(
+                __u('Master password too short'),
+                SPException::ERROR,
+                sprintf(__u('The Master Password length need to be at least %d characters'), self::MIN_LENGTH)
+            );
+        }
+    }
+
     /**
      * Re-encrypts everything under a new master password, or leaves it all as it was.
      *
@@ -129,6 +159,8 @@ public function changeMasterPassword(UpdateMasterPassRequest $request): void
             throw ServiceException::error(__u('Ey, this is a DEMO!!'));
         }
 
+        self::assertLongEnough($request->getNewMasterPass());
+
         $this->repository->transactionAware(
             function () use ($request) {
                 $this->accountMasterPasswordService->updateMasterPassword($request);

src/Application/Install/Services/Installer.php

@@ -27,6 +27,7 @@
 
 namespace SP\Application\Install\Services;
 
+use SP\Application\Crypt\Services\MasterPass;
 use Exception;
 use SP\Application\Config\Ports\ConfigFileService;
 use SP\Application\Config\Ports\ConfigService;
@@ -145,13 +146,7 @@ private function checkData(): void
             );
         }
 
-        if (strlen($this->installData->getMasterPassword() ?? '') < 11) {
-            throw new InvalidArgumentException(
-                __u('Master password too short'),
-                SPException::CRITICAL,
-                __u('The Master Password length need to be at least 11 characters')
-            );
-        }
+        MasterPass::assertLongEnough($this->installData->getMasterPassword());
 
         if ($this->installData->getMasterPassword() !== $this->installData->getMasterPasswordRepeat()) {
             throw new InvalidArgumentException(

src/Infrastructure/Adapter/In/Web/Controllers/ConfigEncryption/SaveController.php

@@ -42,6 +42,7 @@
 use SP\Domain\Crypt\Dtos\UpdateMasterPassRequest;
 use SP\Application\Crypt\Ports\MasterPassService;
 use SP\Application\Crypt\Services\MasterPass;
+use SP\Domain\Core\Exceptions\InvalidArgumentException;
 use SP\Domain\Core\Exceptions\NoSuchItemException;
 use SP\Infrastructure\Adapter\In\Web\Controllers\SimpleControllerBase;
 use SP\Infrastructure\Adapter\In\Web\Controllers\Helpers\SimpleControllerHelper;
@@ -100,6 +101,14 @@ public function saveAction(): ActionResponse
             return ActionResponse::error(__u('Master passwords do not match'));
         }
 
+        // Asked here as well as in the rotation, because the "hash only" option below never
+        // reaches it.
+        try {
+            MasterPass::assertLongEnough($newMasterPass);
+        } catch (InvalidArgumentException $e) {
+            return ActionResponse::error($e->getMessage(), $e->getHint());
+        }
+
         if (!$this->masterPassService->checkMasterPassword($currentMasterPass)) {
             return ActionResponse::error(__u('The current master password does not match'));
         }

tests/Integration/Infrastructure/Adapter/In/Web/Controllers/ConfigEncryption/SaveRefusalsTest.php

@@ -191,6 +191,40 @@ public function aDemoInstanceRefusesIt()
         $this->whenSaving($this->form(), demo: true);
     }
 
+    /**
+     * A new master password is held to the minimum the installer set, here as there. Every secret
+     * is sealed with it, so this was a way to re-key the whole vault to `a`.
+     *
+     * @throws ContainerExceptionInterface
+     * @throws Exception
+     * @throws NotFoundExceptionInterface
+     */
+    #[Test]
+    #[BodyChecker('outputCheckerTooShort')]
+    public function aNewPasswordShorterThanTheMinimumIsRefused()
+    {
+        $this->whenSaving($this->form(['new_masterpass' => 'short_pass', 'new_masterpass_repeat' => 'short_pass']));
+    }
+
+    /**
+     * Including by the "hash only" option, which replaces the stored hash without the rotation
+     * and so never reaches the service's own check.
+     *
+     * @throws ContainerExceptionInterface
+     * @throws Exception
+     * @throws NotFoundExceptionInterface
+     */
+    #[Test]
+    #[BodyChecker('outputCheckerTooShort')]
+    public function aShortPasswordIsRefusedWhenOnlyTheHashChanges()
+    {
+        $this->whenSaving(
+            $this->form(
+                ['new_masterpass' => 'short_pass', 'new_masterpass_repeat' => 'short_pass', 'no_account_change' => 'true']
+            )
+        );
+    }
+
     /**
      * The form as it is submitted when everything is right.
      *
@@ -258,6 +292,11 @@ private function outputCheckerNotConfirmed(string $output): void
         self::assertSame('The password update must be confirmed', json_decode($output)->description);
     }
 
+    private function outputCheckerTooShort(string $output): void
+    {
+        self::assertSame('Master password too short', json_decode($output)->description);
+    }
+
     private function outputCheckerSame(string $output): void
     {
         self::assertSame('Passwords are the same', json_decode($output)->description);

tests/Unit/Application/Crypt/Services/MasterPassTest.php

@@ -38,6 +38,7 @@
 use SP\Application\Config\Ports\ConfigService;
 use SP\Domain\Core\Exceptions\ConstraintException;
 use SP\Domain\Core\Exceptions\QueryException;
+use SP\Domain\Core\Exceptions\InvalidArgumentException;
 use SP\Domain\Crypt\Dtos\UpdateMasterPassRequest;
 use SP\Application\Crypt\Services\MasterPass;
 use SP\Application\CustomField\Ports\CustomFieldCryptService;
@@ -172,7 +173,7 @@ public function testChangeMasterPassword()
             ->method('transactionAware')
             ->with(self::withResolveCallableCallback());
 
-        $request = new UpdateMasterPassRequest('123', '456', $hash);
+        $request = new UpdateMasterPassRequest('123', 'a_new_master_pass', $hash);
 
         $this->accountMasterPasswordService
             ->expects(self::once())
@@ -215,7 +216,7 @@ public function testChangeMasterPassword()
      */
     public function testTheStoredHashIsWrittenInsideTheTransaction(): void
     {
-        $request = new UpdateMasterPassRequest('123', '456', self::$faker->sha1());
+        $request = new UpdateMasterPassRequest('123', 'a_new_master_pass', self::$faker->sha1());
 
         // No withResolveCallableCallback(): the closure is handed over and never invoked.
         $this->repository
@@ -245,7 +246,7 @@ public function testChangeMasterPasswordAbortedOnError(): void
             ->method('transactionAware')
             ->with(self::withResolveCallableCallback());
 
-        $request = new UpdateMasterPassRequest('123', '456', $hash);
+        $request = new UpdateMasterPassRequest('123', 'a_new_master_pass', $hash);
 
         $this->accountMasterPasswordService
             ->expects(self::once())
@@ -310,7 +311,7 @@ public function testADemoInstanceRefusesTheRotation(): void
         $this->expectException(ServiceException::class);
         $this->expectExceptionMessage('Ey, this is a DEMO!!');
 
-        $this->masterPass->changeMasterPassword(new UpdateMasterPassRequest('old', 'new', self::$faker->sha1()));
+        $this->masterPass->changeMasterPassword(new UpdateMasterPassRequest('old', 'a_new_master_pass', self::$faker->sha1()));
     }
 
     /**
@@ -342,7 +343,7 @@ static function (string $param) use (&$saved): bool {
         );
 
         try {
-            $this->masterPass->changeMasterPassword(new UpdateMasterPassRequest('old', 'new', self::$faker->sha1()));
+            $this->masterPass->changeMasterPassword(new UpdateMasterPassRequest('old', 'a_new_master_pass', self::$faker->sha1()));
         } catch (ServiceException) {
             // asserted above; this test is about what reached the config
         }
@@ -368,4 +369,34 @@ protected function setUp(): void
             $this->repository
         );
     }
+    /**
+     * The minimum is counted in characters, as its message promises: four CJK characters are
+     * twelve bytes, which a byte count let past a rule of eleven.
+     */
+    public function testTheMinimumLengthIsCountedInCharacters(): void
+    {
+        MasterPass::assertLongEnough(str_repeat('a', MasterPass::MIN_LENGTH));
+
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage('Master password too short');
+
+        MasterPass::assertLongEnough('密码很长的');
+    }
+
+    /**
+     * The rotation refuses a new master password under the minimum before it touches anything —
+     * `sp:updateMasterPassword` reaches it with no other check on the way.
+     *
+     * @throws ServiceException
+     */
+    public function testChangeMasterPasswordRefusesAShortPasswordBeforeReEncryptingAnything(): void
+    {
+        $this->accountMasterPasswordService->expects(self::never())->method('updateMasterPassword');
+        $this->customFieldCryptService->expects(self::never())->method('updateMasterPassword');
+
+        $this->expectException(InvalidArgumentException::class);
+
+        $this->masterPass->changeMasterPassword(new UpdateMasterPassRequest('old', 'short', self::$faker->sha1()));
+    }
+
 }