Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions src/Application/Account/Services/Account.php
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,39 @@ public function getById(int $id): AccountModel
*
* @return bool
*/
/**
* The privacy flags a restore may write, decided the way the edit screen decides them.
*
* 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.
*
* @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);

return $dto->mutate(
[
'isPrivate' => $mayBePrivate ? $dto->isPrivate : 0,
'isPrivateGroup' => $mayBePrivateGroup ? $dto->isPrivateGroup : 0,
]
);
}

protected function userCanChangeOwner(
UserDto $userData,
ProfileData $userProfile,
Expand Down Expand Up @@ -595,6 +628,18 @@ function () use ($dto) {
$changeUserGroup = $this->userCanChangeGroup($userData, $userProfile, $account);
}

// 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
// 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);

$this->addHistory($dto->accountId);

$result = $this->accountRepository->restoreModified(
Expand Down
56 changes: 54 additions & 2 deletions tests/Unit/Application/Account/Services/AccountTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1226,11 +1226,15 @@ public function testRestoreModified()
$this->accountHistoryService->expects(self::once())->method('create')
->with($accountHistoryCreateDto);

// The privacy flags a restore may write are decided the way the edit screen decides them,
// not taken from the snapshot — the harness user is an ordinary one, so both are refused.
$restored = $accountHistoryDto->mutate(['isPrivate' => 0, 'isPrivateGroup' => 0]);

$this->accountRepository->expects(self::once())->method('restoreModified')
->with(
$accountHistoryDto->accountId,
AccountModel::restoreModified(
$accountHistoryDto,
$restored,
$this->context->getUserData()->id
)
)
Expand All @@ -1239,6 +1243,51 @@ public function testRestoreModified()
$this->account->restoreModified($accountHistoryDto);
}

/**
* A restore cannot mark an account private for somebody who could not mark it private.
*
* `AccountForm::constrainPrivacyToPermission()` gates both flags on the edit screen — only an
* application administrator, or the owner holding `isAccPrivate()` — and nothing re-applied it
* on the way back from a history row. `SaveEditRestoreController` checks only
* `ACCOUNT_EDIT_RESTORE`, which `AccountPermission` buckets with plain edit access, so anybody
* the account was shared with for editing could restore an old version to hide it. `AccountAcl`
* tests privacy *before* the administrator branch, so a private account disappears for account
* administrators too.
*
* The same shape as the owner and group flags one field over, which this method already
* refuses to take from the snapshot.
*
* @throws Exception
*/
public function testRestoreModifiedCannotMakeAnAccountPrivate()
{
$this->configService->method('getByParam')->willReturn(self::$faker->password());

$accountDataGenerator = AccountDataGenerator::factory();

$this->accountRepository->method('getById')
->willReturn(new QueryResult([$accountDataGenerator->buildAccount()]));

$accountHistoryDto = $accountDataGenerator->buildAccountHistoryDto()
->mutate(['isPrivate' => 1, 'isPrivateGroup' => 1]);

$this->accountRepository
->expects(self::once())
->method('restoreModified')
->with(
self::anything(),
self::callback(
static fn(AccountModel $account): bool => (int)$account->getIsPrivate() === 0
&& (int)$account->getIsPrivateGroup() === 0
),
self::anything(),
self::anything()
)
->willReturn(new QueryResult(null, 1));

$this->account->restoreModified($accountHistoryDto);
}

/**
* @throws ServiceException
*/
Expand All @@ -1263,11 +1312,14 @@ public function testRestoreModifiedError()
->with($accountHistoryCreateDto);


// As above: the privacy flags are decided rather than taken from the snapshot.
$restored = $accountHistoryDto->mutate(['isPrivate' => 0, 'isPrivateGroup' => 0]);

$this->accountRepository->expects(self::once())->method('restoreModified')
->with(
$accountHistoryDto->accountId,
AccountModel::restoreModified(
$accountHistoryDto,
$restored,
$this->context->getUserData()->id
)
)
Expand Down