Flag of Ukraine
SymfonyCasts stands united with the people of Ukraine
TRACK

APIs >

COURSE

API Platform 3 Part 2: Security for your Treasures

Buy Access to Course

Validating how Values Change

English Español

Keep on Learning!

If you liked what you've learned so far, dive in!
Subscribe to get access to this tutorial plus
video, code and script downloads.

Start your All-Access Pass
Buy just this tutorial for $12.00
Previous Chapter

Next Chapter
Edit on GitHub

With a Subscription, click any sentence in the script to jump to that part of the video!

Login Subscribe

We still have a massive problem making sure treasures don't end up stolen! We just covered the main case: if you make a POST or a PUT request to a treasure endpoint, thanks to our new validation, we make sure you assign the owner to yourself, unless you're an admin. Yay!

But in our API, when POSTing or PATCHing to a user endpoint, you are allowed to send a dragonTreasures field. This, unfortunately allows treasures to be stolen. Simply send a PATCH request to modify your own User record... then set the dragonTreasures field to an array containing the IRI strings of some treasures that you do not own. Whoops!

The easiest solution would be to... make the field not writable. So, inside of User, for dragonTreasures, we would keep this readable, but remove the write group. That would force everyone to use the /api/treasures endpoints to manage their treasures.

The Trickiness of this Problem

If you do want to keep the writable dragonTreasures field... you can, but this problem is tricky to solve.

Let's think: if you send a dragonTreasures field that contains the IRI of a treasure you do not own, that should trigger a validation error. Ok... so maybe we add a validation constraint above this property? The problem is that, by the time that validation runs, the treasures sent over in the JSON have already been set onto this dragonTreasures property. And importantly, the owner on those treasures has already been updated to this User!

Remember: when the serializer sees a DragonTreasure that is not already owned by this user, it will call addDragonTreasure()... which then calls setOwner($this). So, by the time validation runs, it's going to look like we are the owner of the treasure... even though we originally weren't!

Using Previous Data?

What can we do? Well, API Platform does have a concept of "previous data". API Platform clones the data before deserializing the new JSON onto it, which means it is possible to get what the User object originally looked like.

Unfortunately, that clone is shallow, meaning that it clones scalar fields - like username - but any objects - like the DragonTreasure objects are not cloned. There's no way via API Platform to see what they originally looked like.

Testing for the Bug

So, we are going to solve this with validation... but with the help of a special class from Doctrine called the UnitOfWork.

Alrighty, let's whip up a test to shine a light on this pesky bug. Inside tests/Functional/, open UserResourceTest. Copy the previous test, paste, and call it testTreasuresCannotBeStolen(). Create a second user with UserFactory::createOne()... and we need a DragonTreasure that we're going to try to steal. Assign its owner to $otherUser:

69 lines tests/Functional/UserResourceTest.php
... lines 1 - 4
use App\Factory\DragonTreasureFactory;
... lines 6 - 8
class UserResourceTest extends ApiTestCase
{
... lines 11 - 48
public function testTreasuresCannotBeStolen(): void
{
$user = UserFactory::createOne();
$otherUser = UserFactory::createOne();
$dragonTreasure = DragonTreasureFactory::createOne(['owner' => $otherUser]);
... lines 54 - 66
}
}

Let's do this! We log in as $user, update ourselves - which is allowed - then, for the JSON, sure, maybe we still send username... but we also send dragonTreasures set to an array with /api/treasures/ and $dragonTreasure->getId().

At the bottom, assert that this returns a 422:

69 lines tests/Functional/UserResourceTest.php
... lines 1 - 4
use App\Factory\DragonTreasureFactory;
... lines 6 - 8
class UserResourceTest extends ApiTestCase
{
... lines 11 - 48
public function testTreasuresCannotBeStolen(): void
{
$user = UserFactory::createOne();
$otherUser = UserFactory::createOne();
$dragonTreasure = DragonTreasureFactory::createOne(['owner' => $otherUser]);
$this->browser()
->actingAs($user)
->patch('/api/users/' . $user->getId(), [
'json' => [
'username' => 'changed',
'dragonTreasures' => [
'/api/treasures/' . $dragonTreasure->getId(),
],
],
'headers' => ['Content-Type' => 'application/merge-patch+json']
])
->assertStatus(422);
}
}

Ok! Copy the method name. We're expecting this to fail:

symfony php bin/phpunit --filter=testTreasuresCannotBeStolen

And... it does! Status code 200, which means we are allowing treasure to be stolen! Gasp!

Creating the Validator

Ok, let's cook up a new validator class:

php ./bin/console make:validator

Call it TreasuresAllowedOwnerChange.

Go use this immediately. Above the dragonTreasures property, add #[TreasuresAllowedOwnerChange]:

298 lines src/Entity/User.php
... lines 1 - 15
use App\Validator\TreasuresAllowedOwnerChange;
... lines 17 - 69
class User implements UserInterface, PasswordAuthenticatedUserInterface
{
... lines 72 - 107
#[TreasuresAllowedOwnerChange]
private Collection $dragonTreasures;
... lines 110 - 296
}

Next, over in src/Validator/, open up the validator class. We'll do some basic cleanup: use the assert() function to assert that $constraint is an instance of TreasuresAllowedOwnerChange. And also assert that value is an instance of Collection from Doctrine:

28 lines src/Validator/TreasuresAllowedOwnerChangeValidator.php
... lines 1 - 8
class TreasuresAllowedOwnerChangeValidator extends ConstraintValidator
{
public function validate($value, Constraint $constraint)
{
assert($constraint instanceof TreasuresAllowedOwnerChange);
if (null === $value || '' === $value) {
return;
}
// meant to be used above a Collection field
assert($value instanceof Collection);
... lines 21 - 25
}
}

We know that this will be used above this property... so it will be some sort of collection of DragonTreasures.

Enter UnitOfWork

But... this will be the collection of DragonTreasure objects after they've been modified. We need to ask Doctrine what each DragonTreasure looked like when it was originally queried from the database. To do that, we need to grab an internal object from Doctrine called the UnitOfWork.

On top, add a constructor, autowire EntityManagerInterface $entityManager... and make that's a private property:

42 lines src/Validator/TreasuresAllowedOwnerChangeValidator.php
... lines 1 - 6
use Doctrine\ORM\EntityManagerInterface;
... lines 8 - 10
class TreasuresAllowedOwnerChangeValidator extends ConstraintValidator
{
public function __construct(private EntityManagerInterface $entityManager)
{
}
... lines 16 - 40
}

Below, grab the unit of work with $unitOfWork = $this->entityManager->getUnitOfWork():

42 lines src/Validator/TreasuresAllowedOwnerChangeValidator.php
... lines 1 - 10
class TreasuresAllowedOwnerChangeValidator extends ConstraintValidator
{
... lines 13 - 16
public function validate($value, Constraint $constraint)
{
... lines 19 - 24
// meant to be used above a Collection field
assert($value instanceof Collection);
$unitOfWork = $this->entityManager->getUnitOfWork();
... lines 29 - 39
}
}

This is a powerful object that keeps track of how entity objects are changing and is responsible for knowing which objects need to be inserted, updated or deleted from the database when the entity manger flushes.

Next, foreach over $value - which will be a collection - as $dragonTreasure. To help my editor, I'll assert that $dragonTreasure is an instance of DragonTreasure. And now, get the original data: $originalData = $unitOfWork->getOriginalEntityData($dragonTreasure).

Pretty sweet right? Let's dd($dragonTreasure) and $originalData so we can see what they look like:

42 lines src/Validator/TreasuresAllowedOwnerChangeValidator.php
... lines 1 - 10
class TreasuresAllowedOwnerChangeValidator extends ConstraintValidator
{
... lines 13 - 16
public function validate($value, Constraint $constraint)
{
... lines 19 - 27
$unitOfWork = $this->entityManager->getUnitOfWork();
foreach ($value as $dragonTreasure) {
assert($dragonTreasure instanceof DragonTreasure);
$originalData = $unitOfWork->getOriginalEntityData($dragonTreasure);
dd($dragonTreasure, $originalData);
}
... lines 35 - 39
}
}

Go test go:

symfony php bin/phpunit --filter=testTreasuresCannotBeStolen

Yes! It hit the dump! And this is cool! The first part is the updated DragonTreasure object and its owner has ID 1. It's not super obvious, but $user will be id 1 and $otherUser will be id 2. So the owner was originally ID 2, but yeah: user id 1 has stolen it! Below this, we see the original data as an array. And its owner was ID 2!

This info makes us dangerous. Back inside our validator, say $originalOwnerId = originalData['owner_id']. And to be super clear, set $newOwnerId to $dragonTreasure->getOwner()->getId().

If these don't match, we have a problem. Well actually, if we don't have an $originalOwnerId, we're creating a new DragonTreasure and that's ok. So if there is no $originalOwnerId or the $originalOwnerId is equal to the $newOwnerId, we're good!

Else... there's some plundering happening! Move the $violationBuilder up, but remove setParameter():

46 lines src/Validator/TreasuresAllowedOwnerChangeValidator.php
... lines 1 - 10
class TreasuresAllowedOwnerChangeValidator extends ConstraintValidator
{
... lines 13 - 16
public function validate($value, Constraint $constraint)
{
... lines 19 - 27
$unitOfWork = $this->entityManager->getUnitOfWork();
foreach ($value as $dragonTreasure) {
assert($dragonTreasure instanceof DragonTreasure);
$originalData = $unitOfWork->getOriginalEntityData($dragonTreasure);
$originalOwnerId = $originalData['owner_id'];
$newOwnerId = $dragonTreasure->getOwner()->getId();
if (!$originalOwnerId || $originalOwnerId === $newOwnerId) {
return;
}
// the owner is being changed
$this->context->buildViolation($constraint->message)
->addViolation();
}
}
}

That's it!

Oh, but I never customized the error message. In the Constraint class, give the $message property a better default message:

21 lines src/Validator/TreasuresAllowedOwnerChange.php
... lines 1 - 12
class TreasuresAllowedOwnerChange extends Constraint
{
... lines 15 - 18
public string $message = 'One of the treasures illegally changed owners.';
}

All right team, moment of truth! Run that test:

symfony php bin/phpunit --filter=testTreasuresCannotBeStolen

Nailed it! Treasure stealing is officially off the table. Oh, and though I didn't do it, we could also inject the Security service to allow admin users to do whatever they want.

Up next: when we create a DragonTreasure, we must send the owner field. Let's finally make that optional. If we don't pass the owner, we'll set it to the currently authenticated user. To do that, we need to hook into API platform's "saving" process one more time.

Leave a comment!

Most Useful Newest Oldest
2
Login or Register to join the conversation
Michel Avatar

I think the return in the foreach loop should be a continue. Otherwise it will only check if the owner of the first dragon treasure is correct and if so, it will not check the other dragon treasures.

3 Reply
MolloKhan Avatar

Hey @Michel

You're right, it won't check any other DragonTreasures, however, that's a problem only if you allow updating multiple objects at once, which we don't currently support. So, for now, it will just work :)

Cheers!

1 Reply
Cat in space

"Houston: no signs of life"
Start the conversation!

API Platform 3 Symfony 6.2

What PHP libraries does this tutorial use?

// composer.json
{
    "require": {
        "php": ">=8.1",
        "ext-ctype": "*",
        "ext-iconv": "*",
        "api-platform/core": "^3.0", // v3.1.2
        "doctrine/annotations": "^2.0", // 2.0.1
        "doctrine/doctrine-bundle": "^2.8", // 2.8.3
        "doctrine/doctrine-migrations-bundle": "^3.2", // 3.2.2
        "doctrine/orm": "^2.14", // 2.14.1
        "nelmio/cors-bundle": "^2.2", // 2.2.0
        "nesbot/carbon": "^2.64", // 2.66.0
        "phpdocumentor/reflection-docblock": "^5.3", // 5.3.0
        "phpstan/phpdoc-parser": "^1.15", // 1.16.1
        "symfony/asset": "6.2.*", // v6.2.5
        "symfony/console": "6.2.*", // v6.2.5
        "symfony/dotenv": "6.2.*", // v6.2.5
        "symfony/expression-language": "6.2.*", // v6.2.5
        "symfony/flex": "^2", // v2.2.4
        "symfony/framework-bundle": "6.2.*", // v6.2.5
        "symfony/property-access": "6.2.*", // v6.2.5
        "symfony/property-info": "6.2.*", // v6.2.5
        "symfony/runtime": "6.2.*", // v6.2.5
        "symfony/security-bundle": "6.2.*", // v6.2.6
        "symfony/serializer": "6.2.*", // v6.2.5
        "symfony/twig-bundle": "6.2.*", // v6.2.5
        "symfony/ux-react": "^2.6", // v2.7.1
        "symfony/ux-vue": "^2.7", // v2.7.1
        "symfony/validator": "6.2.*", // v6.2.5
        "symfony/webpack-encore-bundle": "^1.16", // v1.16.1
        "symfony/yaml": "6.2.*" // v6.2.5
    },
    "require-dev": {
        "doctrine/doctrine-fixtures-bundle": "^3.4", // 3.4.2
        "mtdowling/jmespath.php": "^2.6", // 2.6.1
        "phpunit/phpunit": "^9.5", // 9.6.3
        "symfony/browser-kit": "6.2.*", // v6.2.5
        "symfony/css-selector": "6.2.*", // v6.2.5
        "symfony/debug-bundle": "6.2.*", // v6.2.5
        "symfony/maker-bundle": "^1.48", // v1.48.0
        "symfony/monolog-bundle": "^3.0", // v3.8.0
        "symfony/phpunit-bridge": "^6.2", // v6.2.5
        "symfony/stopwatch": "6.2.*", // v6.2.5
        "symfony/web-profiler-bundle": "6.2.*", // v6.2.5
        "zenstruck/browser": "^1.2", // v1.2.0
        "zenstruck/foundry": "^1.26" // v1.28.0
    }
}
Edit on GitHub
Previous Chapter
Next Chapter
userVoice