api-platform / api-platform/core

[Security] Document is persisted before voters result on securityPostDenormalize

Open
#8,429 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
PHP
Stars
2.6k
Forks
980
Avg merge
2d 4h
Merged PRs (30d)
49

Description

**API Platform version(s) affected**: 4.3.5

**Description**
When a resource is behind a voter on Patch operation and custom logic is called with a Voter using `securityPostDenormalize`, the incoming document is still persisted and therefore is written in the database if a flush comes after.

AgentRole.php
```php
#[ApiResource(operations: [
new Patch(
securityPostDenormalize: "is_granted('ROLE_UPDATE', object)",
extraProperties: ['throw_on_access_denied' => true],
),
])]
```

AgentRoleVoter.php
```php
protected function supports(string $attribute, mixed $subject): bool
{
$supportAttributes = $attribute == 'ROLE_UPDATE';
$supportSubject = $subject instanceof AgentRole;

return $supportAttributes && $supportSubject;
}

protected function voteOnAttribute(string $attribute, mixed $subject, TokenInterface $token): bool
{
// some false logic
}
```

AgentRoleTest.php
```php
use ResetDatabase;

public function testChangeAgentRoleToDev()
{
$client = static::createClientWithCredentials();
$dm = $this->getContainer()->get(DocumentManager::class);

$agentRoles = AgentRoleFactory::createSequence([
['agentId' => '1', 'roles' => ['ROLE_USER']],
['agentId' => '2', 'roles' => ['ROLE_USER']]
]);

$client->request('PATCH', '/agent_roles/1', [
'headers' => ['Content-Type' => 'application/merge-patch+json'],
'json' => [
'roles' => ['ROLE_USER', 'ROLE_DEV']
]
]);

$this->assertResponseStatusCodeSame(403); // OK

// I would need to use $dm->clear() to flush previous operation without persisting
// $dm->clear();

$currentUserRole = $dm->getRepository(AgentRole::class)->findOneBy([
'agentId' => 'user' // current logged user
]);
$currentUserRole->setRoles(['ROLE_ADMIN']);
$dm->persist($currentUserRole);
$dm->flush(); // <-------- this persist also the request operation which wasn't persisted on request execution
}
```

Before flush:
Image

After:
Image

Contributor guide

Open the contributing guide

Research direction

Start with the securityPostDenormalize processing and the AgentRoleTest.php reproduction, then trace how the DocumentManager handles the denied PATCH object. Run the shown test scenario and verify that a later flush does not persist changes from the failed request while still persisting the explicitly changed current-user role.

Written by the indexing model from the issue text.

Assessment

Tech stack
php
Domain
authorization, databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.