OpusProjects · blaipr · Sep 24, 2026 · Sep 24, 2026
diff --git a/CLAUDE.md b/CLAUDE.md
@@ -471,6 +471,9 @@ user and account endpoints did neither. Custom-field values were masked for a ca
 enforced in five web config actions and mentioned nowhere on the API at all — the sharpest case,
 because a demo publishes its administrator's credentials, so the ACL stops nobody and that guard is
 the whole boundary. Search paging clamped a negative offset in one of the two DTOs that carry one.
+Who may mark an account private was decided in `AccountForm`, so the API's account create and edit
+wrote `private` / `privateGroup` as sent; it is now `Account::privacyAllowedFor()`, for the owner
+the account will actually have.
 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

diff --git a/src/Application/Account/Services/Account.php b/src/Application/Account/Services/Account.php
@@ -295,29 +295,42 @@ public function getById(int $id): AccountModel
      * @return bool
      */
     /**
-     * The privacy flags a restore may write, decided the way the edit screen decides them.
+     * Whether the signed-in user may mark an account owned by `$ownerId` / `$groupId` private to its
+     * owner and to its group — the one definition every write asks.
      *
-     * Mirrors `AccountForm::constrainPrivacyToPermission()`. Both flags only ever *withhold* an
-     * account, so this is not a way to reach anything — it is a way to hide one, or to stop hiding
-     * one, without the permission that governs it.
+     * Only an application administrator may, or the owner whose profile holds `isAccPrivate()` (the
+     * main group, `isAccPrivateGroup()`). Both flags only ever *withhold* an account, so this is not
+     * a way to reach anything; it is a way to hide one, or to stop hiding one, without the
+     * permission that governs it — and `AccountAcl` tests privacy before the administrator branch,
+     * so a private account disappears for account administrators as well.
+     *
+     * It used to be decided in `AccountForm`, which only the web reaches: `POST account/create` and
+     * `account/edit` on the API put the caller's `private` / `privateGroup` straight into the row.
+     *
+     * @return array{0: bool, 1: bool} May be private, may be private to the group
+     */
+    private function privacyAllowedFor(?int $ownerId, ?int $groupId): array
+    {
+        $userData = $this->context->getUserData();
+        $userProfile = $this->context->getUserProfile() ?? new ProfileData();
+
+        return [
+            $userData->isAdminApp || ($userProfile->isAccPrivate() && $ownerId === $userData->id),
+            $userData->isAdminApp
+            || ($userProfile->isAccPrivateGroup() && $groupId === $userData->userGroupId),
+        ];
+    }
+
+    /**
+     * The privacy flags a restore may write, decided the way every other write decides them.
      *
      * @param AccountHistoryDto $dto
-     * @param UserDto $userData
-     * @param ProfileData $userProfile
      *
      * @return AccountHistoryDto
      */
-    private function constrainPrivacyToPermission(
-        AccountHistoryDto $dto,
-        UserDto           $userData,
-        ProfileData       $userProfile
-    ): AccountHistoryDto {
-        $mayBePrivate = $userData->isAdminApp
-                        || ($userProfile->isAccPrivate() && $dto->userId === $userData->id);
-
-        $mayBePrivateGroup = $userData->isAdminApp
-                             || ($userProfile->isAccPrivateGroup()
-                                 && $dto->userGroupId === $userData->userGroupId);
+    private function constrainPrivacyToPermission(AccountHistoryDto $dto): AccountHistoryDto
+    {
+        [$mayBePrivate, $mayBePrivateGroup] = $this->privacyAllowedFor($dto->userId, $dto->userGroupId);
 
         return $dto->mutate(
             [
@@ -327,6 +340,32 @@ private function constrainPrivacyToPermission(
         );
     }
 
+    /**
+     * The privacy flags a create or an edit may write, for the account's effective owner and group.
+     *
+     * @template T of AccountCreateDto|AccountUpdateDto
+     * @param T $dto
+     *
+     * @return T
+     */
+    private function constrainAccountPrivacy(
+        AccountCreateDto|AccountUpdateDto $dto,
+        ?int $ownerId,
+        ?int $groupId
+    ): AccountCreateDto|AccountUpdateDto {
+        [$mayBePrivate, $mayBePrivateGroup] = $this->privacyAllowedFor($ownerId, $groupId);
+
+        if (!$mayBePrivate) {
+            $dto = $dto->withPrivate(false);
+        }
+
+        if (!$mayBePrivateGroup) {
+            $dto = $dto->withPrivateGroup(false);
+        }
+
+        return $dto;
+    }
+
     protected function userCanChangeOwner(
         UserDto $userData,
         ProfileData  $userProfile,
@@ -402,6 +441,11 @@ function () use ($accountCreateDto) {
                     $this->accountCryptService->getPasswordEncrypted($accountCreateDto->pass)
                 );
 
+                $accountCreateDto = $this->constrainAccountPrivacy(
+                    $accountCreateDto,
+                    $accountCreateDto->userId,
+                    $accountCreateDto->userGroupId
+                );
                 $accountCreateDto = $this->setPresetPrivate($accountCreateDto);
 
                 $accountId = $this->accountRepository->create(AccountModel::create($accountCreateDto))->getLastId();
@@ -501,12 +545,20 @@ function () use ($id, $accountUpdateDto) {
                         );
                     }
                 } else {
+                    $account = $this->getById($id);
                     $changeOwner = false;
                     $changeUserGroup = false;
                 }
 
                 $this->addHistory($id);
 
+                // Decided for whoever will own the account once this is saved: an owner or group
+                // the caller may not change is not changed, whatever the request said.
+                $accountUpdateDto = $this->constrainAccountPrivacy(
+                    $accountUpdateDto,
+                    $changeOwner ? $accountUpdateDto->userId : $account->getUserId(),
+                    $changeUserGroup ? $accountUpdateDto->userGroupId : $account->getUserGroupId()
+                );
                 $accountUpdateDto = $this->setPresetPrivate($accountUpdateDto, $id);
 
                 if ($accountUpdateDto->userEditId === null) {
@@ -631,14 +683,14 @@ function () use ($dto) {
                 // And the two privacy flags, which are the same question asked about a different
                 // pair of columns and were still being taken from the snapshot.
                 //
-                // `AccountForm::constrainPrivacyToPermission()` decides this on the edit screen:
-                // only an application administrator, or the owner holding `isAccPrivate()` (the
-                // group holding `isAccPrivateGroup()`), may set them. Nothing re-applied it on the
+                // `privacyAllowedFor()` decides this for a create or an edit: only an application
+                // administrator, or the owner holding `isAccPrivate()` (the group holding
+                // `isAccPrivateGroup()`), may set them. Nothing re-applied it on the
                 // way back from a history row, so a restore let anybody with edit rights mark an
                 // account private — and `AccountAcl` tests privacy *before* the administrator
                 // branch, so a private account disappears for account administrators too — or strip
                 // the privacy from one that had it.
-                $dto = $this->constrainPrivacyToPermission($dto, $userData, $userProfile);
+                $dto = $this->constrainPrivacyToPermission($dto);
 
                 $this->addHistory($dto->accountId);
 

diff --git a/src/Infrastructure/Adapter/In/Web/Forms/AccountForm.php b/src/Infrastructure/Adapter/In/Web/Forms/AccountForm.php
@@ -139,52 +139,11 @@ private function analyzeRequestData(): AccountCreateDto|AccountUpdateDto
             'userGroupId' => $this->request->analyzeInt('main_usergroup_id'),
         ];
 
-        $properties = $this->constrainPrivacyToPermission($properties);
-
         return $this->itemId === null ? AccountCreateDto::fromArray($properties) : AccountUpdateDto::fromArray(
             $properties
         );
     }
 
-    /**
-     * Hold the two privacy flags to the same rule the interface applies when it decides whether to
-     * offer them.
-     *
-     * `AccountHelper` computes `allowPrivate` / `allowPrivateGroup` from the profile permission and
-     * ownership, and the template omits the checkbox when they are false — but nothing looked at
-     * either on the way back in, so a request that carried `private_enabled` anyway was honoured.
-     * That is not a way to reach anything: both flags only ever *withhold* an account. It is a way
-     * to hide one, and `AccountAcl` tests privacy before the administrator branch, so an account
-     * marked private disappears for account administrators too.
-     *
-     * @param array<string, mixed> $properties
-     *
-     * @return array<string, mixed>
-     */
-    private function constrainPrivacyToPermission(array $properties): array
-    {
-        $userData = $this->context->getUserData();
-        $userProfile = $this->context->getUserProfile();
-
-        $mayBePrivate = $userData->isAdminApp
-                        || (($userProfile?->isAccPrivate() ?? false)
-                            && $properties['userId'] === $userData->id);
-
-        $mayBePrivateGroup = $userData->isAdminApp
-                             || (($userProfile?->isAccPrivateGroup() ?? false)
-                                 && $properties['userGroupId'] === $userData->userGroupId);
-
-        if (!$mayBePrivate) {
-            $properties['isPrivate'] = 0;
-        }
-
-        if (!$mayBePrivateGroup) {
-            $properties['isPrivateGroup'] = 0;
-        }
-
-        return $properties;
-    }
-
     /**
      * @throws ValidationException
      */

diff --git a/tests/Unit/Application/Account/Services/AccountTest.php b/tests/Unit/Application/Account/Services/AccountTest.php
@@ -130,6 +130,10 @@ public function testUpdateUserCannotChangePermissionsWithoutPermission()
         $accountDataGenerator = AccountDataGenerator::factory();
         $accountUpdateDto = $accountDataGenerator->buildAccountUpdateDto();
 
+        // Nobody signed in here may mark an account private, so whatever the request carried, the
+        // account is written with both flags off.
+        $expectedDto = $accountUpdateDto->withPrivate(false)->withPrivateGroup(false);
+
         $this->context->setUserData(
             UserDto::fromModel(
                 UserDataGenerator::factory()
@@ -146,19 +150,107 @@ public function testUpdateUserCannotChangePermissionsWithoutPermission()
         $this->itemPresetService->expects(self::once())->method('getForCurrentUser')
                                 ->with(ItemPresetInterface::ITEM_TYPE_ACCOUNT_PRIVATE)
                                 ->willReturn(null);
-        $this->accountRepository->expects(self::once())->method('getById')
+        // Once for the history, once to know who owns the account the flags are decided for.
+        $this->accountRepository->expects(self::exactly(2))->method('getById')
                                 ->with($id)
                                 ->willReturn(new QueryResult([$accountDataGenerator->buildAccount()]));
         $this->accountRepository->expects(self::once())->method('update')
-            ->with($id, AccountModel::update($accountUpdateDto), false, false)
+            ->with($id, AccountModel::update($expectedDto), false, false)
             ->willReturn(new QueryResult(null, 1));
         $this->accountItemsService->expects(self::once())->method('updateItems')
-                                  ->with(false, $id, $accountUpdateDto);
+                                  ->with(false, $id, $expectedDto);
         $this->accountPresetService->expects(self::once())->method('addPresetPermissions')->with($id);
 
         $this->account->update($id, $accountUpdateDto);
     }
 
+    /**
+     * An owner whose profile may mark accounts private keeps the flags they asked for.
+     *
+     * @throws ServiceException
+     * @throws SPException
+     */
+    public function testUpdateKeepsPrivacyAskedForByAnOwnerWithThePermission()
+    {
+        $id = self::$faker->randomNumber();
+        $accountDataGenerator = AccountDataGenerator::factory();
+        $userData = UserDto::fromModel(
+            UserDataGenerator::factory()->buildUserData()->mutate(['isAdminApp' => false, 'isAdminAcc' => false])
+        );
+
+        $this->context->setUserData($userData);
+        $this->context->setUserProfile(new ProfileData(['accPrivate' => true, 'accPrivateGroup' => true]));
+
+        $accountUpdateDto = $accountDataGenerator->buildAccountUpdateDto()->withPrivate(true)->withPrivateGroup(true);
+        $stored = $accountDataGenerator->buildAccount()->mutate(
+            ['userId' => $userData->id, 'userGroupId' => $userData->userGroupId]
+        );
+
+        $this->configService->method('getByParam')->willReturn(self::$faker->password());
+        $this->itemPresetService->method('getForCurrentUser')->willReturn(null);
+        $this->accountRepository->method('getById')->willReturn(new QueryResult([$stored]));
+        $this->accountRepository->expects(self::once())->method('update')
+            ->with(
+                $id,
+                self::callback(
+                    static fn(AccountModel $account) => $account->getIsPrivate() === 1
+                                                         && $account->getIsPrivateGroup() === 1
+                ),
+                false,
+                false
+            )
+            ->willReturn(new QueryResult(null, 1));
+
+        $this->account->update($id, $accountUpdateDto);
+    }
+
+    /**
+     * The flags are decided for whoever owns the account once it is saved. A caller who may not
+     * change the owner does not become it by naming themselves in the request, so the permission to
+     * make one's *own* accounts private does not reach somebody else's — which the API's
+     * `account/edit` allowed, since it carried `private` / `privateGroup` straight into the row.
+     *
+     * @throws ServiceException
+     * @throws SPException
+     */
+    public function testUpdatePrivacyIsDecidedForTheStoredOwnerNotTheOneRequested()
+    {
+        $id = self::$faker->randomNumber();
+        $accountDataGenerator = AccountDataGenerator::factory();
+        $userData = UserDto::fromModel(
+            UserDataGenerator::factory()->buildUserData()->mutate(['isAdminApp' => false, 'isAdminAcc' => false])
+        );
+
+        $this->context->setUserData($userData);
+        $this->context->setUserProfile(new ProfileData(['accPrivate' => true, 'accPrivateGroup' => true]));
+
+        $accountUpdateDto = $accountDataGenerator->buildAccountUpdateDto()
+                                                 ->withUserId($userData->id)
+                                                 ->withUserGroupId($userData->userGroupId)
+                                                 ->withPrivate(true)
+                                                 ->withPrivateGroup(true);
+        $stored = $accountDataGenerator->buildAccount()->mutate(
+            ['userId' => $userData->id + 1, 'userGroupId' => $userData->userGroupId + 1]
+        );
+
+        $this->configService->method('getByParam')->willReturn(self::$faker->password());
+        $this->itemPresetService->method('getForCurrentUser')->willReturn(null);
+        $this->accountRepository->method('getById')->willReturn(new QueryResult([$stored]));
+        $this->accountRepository->expects(self::once())->method('update')
+            ->with(
+                $id,
+                self::callback(
+                    static fn(AccountModel $account) => $account->getIsPrivate() === 0
+                                                         && $account->getIsPrivateGroup() === 0
+                ),
+                false,
+                false
+            )
+            ->willReturn(new QueryResult(null, 1));
+
+        $this->account->update($id, $accountUpdateDto);
+    }
+
     /**
      * @throws ServiceException
      * @throws SPException
@@ -169,6 +261,10 @@ public function testUpdateUserCanChangePermissionsWithAdminAcc()
         $accountDataGenerator = AccountDataGenerator::factory();
         $accountUpdateDto = $accountDataGenerator->buildAccountUpdateDto();
 
+        // Nobody signed in here may mark an account private, so whatever the request carried, the
+        // account is written with both flags off.
+        $expectedDto = $accountUpdateDto->withPrivate(false)->withPrivateGroup(false);
+
         $this->context->setUserData(
             UserDto::fromModel(
                 UserDataGenerator::factory()
@@ -189,10 +285,10 @@ public function testUpdateUserCanChangePermissionsWithAdminAcc()
                                 ->with($id)
                                 ->willReturn(new QueryResult([$accountDataGenerator->buildAccount()]));
         $this->accountRepository->expects(self::once())->method('update')
-            ->with($id, AccountModel::update($accountUpdateDto), true, true)
+            ->with($id, AccountModel::update($expectedDto), true, true)
             ->willReturn(new QueryResult(null, 1));
         $this->accountItemsService->expects(self::once())->method('updateItems')
-                                  ->with(true, $id, $accountUpdateDto);
+                                  ->with(true, $id, $expectedDto);
         $this->accountPresetService->expects(self::once())->method('addPresetPermissions')->with($id);
 
         $this->account->update($id, $accountUpdateDto);
@@ -208,6 +304,10 @@ public function testUpdateUserCanChangePermissionsWithProfilePermission()
         $accountDataGenerator = AccountDataGenerator::factory();
         $accountUpdateDto = $accountDataGenerator->buildAccountUpdateDto();
 
+        // Nobody signed in here may mark an account private, so whatever the request carried, the
+        // account is written with both flags off.
+        $expectedDto = $accountUpdateDto->withPrivate(false)->withPrivateGroup(false);
+
         $this->context->setUserData(
             UserDto::fromModel(
                 UserDataGenerator::factory()
@@ -228,10 +328,10 @@ public function testUpdateUserCanChangePermissionsWithProfilePermission()
                                 ->with($id)
                                 ->willReturn(new QueryResult([$accountDataGenerator->buildAccount()]));
         $this->accountRepository->expects(self::once())->method('update')
-            ->with($id, AccountModel::update($accountUpdateDto), false, false)
+            ->with($id, AccountModel::update($expectedDto), false, false)
             ->willReturn(new QueryResult(null, 1));
         $this->accountItemsService->expects(self::once())->method('updateItems')
-                                  ->with(true, $id, $accountUpdateDto);
+                                  ->with(true, $id, $expectedDto);
         $this->accountPresetService->expects(self::once())->method('addPresetPermissions')->with($id);
 
         $this->account->update($id, $accountUpdateDto);
@@ -1445,7 +1545,10 @@ public function testCreateCannotChangePermissions()
                                 ->with(ItemPresetInterface::ITEM_TYPE_ACCOUNT_PRIVATE)
                                 ->willReturn(null);
 
-        $encryptedDto = $accountCreateDto->withEncryptedPassword($encryptedPassword);
+        // Not allowed to mark it private, so both flags are written off whatever was asked.
+        $encryptedDto = $accountCreateDto->withEncryptedPassword($encryptedPassword)
+                                         ->withPrivate(false)
+                                         ->withPrivateGroup(false);
 
         $this->accountRepository->expects(self::once())->method('create')
             ->with(self::anAccountStampedNow(AccountModel::create($encryptedDto)))