Security Logic in the Validator

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

Our custom validator is being used... but it doesn't have any real logic yet: it always add a "violation". Meaning, this validator always fails. Let's fix that: let's fail if the owner - that's the $value we're validating - is being set to a User that's different than the currently-authenticated user.

To figure out who's logged in, add public function __construct() and autowire our favorite Security service. I'll hit Alt + Enter and go to "Initialize fields" to create that property and set it.

            
            46 lines
            
            src/Validator/IsValidOwnerValidator.php
        
                                                    ... lines 1 - 9
                            
            class IsValidOwnerValidator extends ConstraintValidator
        
            {
        
                private $security;
        
                public function __construct(Security $security)
        
                {
        
                    $this->security = $security;
        
                }
        
                                                    ... lines 18 - 44
                            
            }

Making sure the User Is Authenticated

For the logic itself, start with $user = $this->security->getUser(). For this particular operation, we've added security to guarantee that the user will be authenticated. But just to be safe - or in case we decide to use this validation constraint on some other operation - let's double-check that the user is logged in: if !$user instanceof User, add a violation.

            
            46 lines
            
            src/Validator/IsValidOwnerValidator.php
        
                                                    ... lines 1 - 18
                            
                public function validate($value, Constraint $constraint)
        
                {
        
                                                    ... lines 21 - 26
                            
                    $user = $this->security->getUser();
        
                    if (!$user instanceof User) {
        
                                                    ... lines 29 - 32
                            
                    }
        
                                                    ... lines 34 - 43
                            
                }
        
                                                    ... lines 45 - 46

I could hardcode the message... but to be a bit fancier - and make it configurable each time we use this constraint - on the annotation class, add a public property called $anonymousMessage set to:

Cannot set owner unless you are authenticated

            
            20 lines
            
            src/Validator/IsValidOwner.php
        
                                                    ... lines 1 - 9
                            
            class IsValidOwner extends Constraint
        
            {
        
                                                    ... lines 12 - 15
                            
                public $message = 'Cannot set owner to a different user';
        
                                                    ... lines 17 - 18
                            
            }

Back in the validator, do the same thing as below: $this->context->buildViolation(), then the message - $constraint->anonymousMessage - and ->addViolation(). Finally, return so the function doesn't keep running: set the validation error and exit.

            
            46 lines
            
            src/Validator/IsValidOwnerValidator.php
        
                                                    ... lines 1 - 9
                            
            class IsValidOwnerValidator extends ConstraintValidator
        
            {
        
                                                    ... lines 12 - 18
                            
                public function validate($value, Constraint $constraint)
        
                {
        
                                                    ... lines 21 - 27
                            
                    if (!$user instanceof User) {
        
                        $this->context->buildViolation($constraint->anonymousMessage)
        
                            ->addViolation();
        
                        return;
        
                    }
        
                                                    ... lines 34 - 43
                            
                }
        
            }

Checking the Owner

At this point, we know the user is authenticated. We also know that the $value variable should be a User object... because we're expecting that the IsValidOwner annotation will be set on a property that contains a User. But... because I love making mistakes... I might someday accidentally put this onto some other property that does not contain a User object. If that happens, no problem! Let's send future me a clear message that I'm doing something... well, kinda silly: if !$value instanceof User, throw new \InvalidArgumentException() with

@IsValidOwner constraint must be put on a property containing a User object

            
            46 lines
            
            src/Validator/IsValidOwnerValidator.php
        
                                                    ... lines 1 - 18
                            
                public function validate($value, Constraint $constraint)
        
                {
        
                                                    ... lines 21 - 34
                            
                    if (!$value instanceof User) {
        
                        throw new \InvalidArgumentException('@IsValidOwner constraint must be put on a property containing a User object');
        
                    }
        
                                                    ... lines 38 - 43
                            
                }
        
                                                    ... lines 45 - 46

I'm not using a violation here because this isn't a user-input problem - it's a programmer bug.

Finally, we know that both $user and $value are User objects. All we need to do now is compare them. So if $value->getId() !== $user->getId(), we have a problem. And yes, you could also compare the objects themselves instead of the id's.

Move the violation code into the if statement and... we're done!

            
            46 lines
            
            src/Validator/IsValidOwnerValidator.php
        
                                                    ... lines 1 - 18
                            
                public function validate($value, Constraint $constraint)
        
                {
        
                                                    ... lines 21 - 39
                            
                    if ($value->getId() !== $user->getId()) {
        
                        $this->context->buildViolation($constraint->message)
        
                            ->addViolation();
        
                    }
        
                }
        
                                                    ... lines 45 - 46

If someone sends the owner IRI of a different User, boom! Validation error! Let's see if our test passes:

php bin/phpunit --filter=testCreateCheeseListing

And... it does!

Allowing Admins to do Anything

The nice thing about this setup is that the owner field is still part of our API. We did that for a very specific reason: we want to make it possible for an admin user to send an owner field set to any user. Right now, our validator would block that. So... let's make it a little bit smarter.

Because we already have the Security object autowired here, jump straight to check for the admin role: if $this->security->isGranted('ROLE_ADMIN'), then return. That will prevent the real owner check from happening below.

            
            51 lines
            
            src/Validator/IsValidOwnerValidator.php
        
                                                    ... lines 1 - 18
                            
                public function validate($value, Constraint $constraint)
        
                {
        
                                                    ... lines 21 - 34
                            
                    // allow admin users to change owners
        
                    if ($this->security->isGranted('ROLE_ADMIN')) {
        
                        return;
        
                    }
        
                                                    ... lines 39 - 48
                            
                }
        
                                                    ... lines 50 - 51

Requiring Owner

A few minutes ago, we talked about how the validator starts by checking to see if the $value is null. That would happen if an API client simply forgets to send the owner field. In that situation, our validator does not add a violation. Instead, it returns... which basically means:

In the eyes of this validator, the value is valid.

We did that because if you want the field to be required, that should be done by adding a separate validation constraint - the NotBlank annotation. The fact that we're missing this is a bug... and let's prove it before we fix it.

In CheeseListingResourceTest, move this $cheesyData up above one more request.

            
            76 lines
            
            tests/Functional/CheeseListingResourceTest.php
        
                                                    ... lines 1 - 9
                            
            class CheeseListingResourceTest extends CustomApiTestCase
        
            {
        
                                                    ... lines 12 - 13
                            
                public function testCreateCheeseListing()
        
                {
        
                                                    ... lines 16 - 24
                            
                    $cheesyData = [
        
                        'title' => 'Mystery cheese... kinda green',
        
                        'description' => 'What mysteries does it hold?',
        
                        'price' => 5000
        
                    ];
        
                                                    ... lines 30 - 44
                            
                }
        
                                                    ... lines 46 - 74
                            
            }

This request is the one that makes sure that, after we log in, we are authorized to use this operation: we get a 400 status code due to a validation error, but not the 403 that we would expect if we were denied access.

Now, pass $cheesyData to that operation. We should still get a 400 response: we're passing some data, but we're still missing the owner field... which should be required.

            
            76 lines
            
            tests/Functional/CheeseListingResourceTest.php
        
                                                    ... lines 1 - 13
                            
                public function testCreateCheeseListing()
        
                {
        
                                                    ... lines 16 - 30
                            
                    $client->request('POST', '/api/cheeses', [
        
                        'json' => $cheesyData,
        
                    ]);
        
                                                    ... lines 34 - 44
                            
                }
        
                                                    ... lines 46 - 76

However, when we run the test:

php bin/phpunit --filter=testCreateCheeseListing

It explodes into a big, giant 500 error! It's trying to insert a record into the database with a null owner_id column.

To fix this, above owner, add the missing @Assert\NotBlank().

            
            209 lines
            
            src/Entity/CheeseListing.php
        
                                                    ... lines 1 - 48
                            
            class CheeseListing
        
            {
        
                                                    ... lines 51 - 95
                            
                /**
        
                                                    ... lines 97 - 100
                            
                 * @Assert\NotBlank()
        
                 */
        
                private $owner;
        
                                                    ... lines 104 - 207
                            
            }

The value must now not be blank and it must be a valid owner. Try the test again:

php bin/phpunit --filter=testCreateCheeseListing

It's green! Next, allowing owner to be part of our API is great for admin users. But it's kind of inconvenient for normal users. Could we allow the owner field to be sent when creating a CheeseListing... but automatically set it to the currently-authenticated user if the field is not sent? And if so, how? That's next.

Leave a comment!

Stephane | posted 1 year ago | edited

If you want to use attributes with PHP 8, you have to modify class IsValidOwner like that :


#[\Attribute]
class IsValidOwner extends Constraint {}

Into CheeseListing class :


use App\Validator as AppAssert;
class CheeseListing
{
    #[AppAssert\IsValidOwner]
    private User $owner;
}

I noted with ApiPlatform 2.6, we have to use $this->assertResponseStatusCodeSame(422, 'not passing the correct owner'); instead of $this->assertResponseStatusCodeSame(400, 'not passing the correct owner');

Victor | SFCASTS | Stephane | posted 1 year ago

Hey Stephane,

Thank you for sharing the code that uses PHP 8 attributes! And thanks for the tip about return status code, I think we will add a note about it

Cheers!

Simon L. | posted 2 years ago

Hi there!

Tell me if I am wrong but even if the validator triggers a violation (and thus a 400 error), the request will still be written into the database.

For example, if a user (not admin) wants to set another user of his choice as author to an existing post, his put request will return a 400 error, with the customized error message, but in the database, the owner will effectively be changed...

In your test, please do not use only $this->assertResponseStatusCodeSame(400), but compare also the previous data to the data in the database after the request has been made.

For example, an assertion like this one:
$user = $em->getRepository(UserObject::class)->findOneBy(['username' => 'XXXX']);
$this->assertEquals($prevUuid, $user->getId());

For my part, I had to add this kind of code into the validator to be sure that the query hasn't changed my data in the database even after a validator violation:
$previousStatus = $this->em->getUnitOfWork()->getOriginalEntityData($value)['status'];
$value->setStatus($previousStatus)

So maybe I did something wrong somewhere, so let me know please :)

weaverryan | SFCASTS | Simon L. | posted 2 years ago | edited

Hey Simon L. !

Hmm. So, this definitely should not be happening. It totally may be happening... but it should - so we need to figure out what's going wrong! :)

First, the problem could just be in the test. I don't know what your full test looks like, but it's possible that your test is using "out of date" data. That's because this is what happens behind the scenes:

A) In the test, when you create the Client object, that boots Symfony's kernel & container
B) When you make a request in the test, that request is made through that kernel. This means that if you have, for example, a CheeseListing object in your test (maybe you created one and are actually testing to see if you can update to a different owner), that is literally the same object that will be "modified" in your request.
C) And so, even though the modified CheeseListing was never saved to the database, the CheeseListing in your test was modified and so your assertion makes it appear that it was modified (but really, the data is coming from "memory" and not from the database).

This may or may not be the issue, but I wanted to mention it first. It's an annoying little thing in tests. To be sure, "refresh" any objects before you run an assertion on them - e.g. $em->refresh($cheeseListing). Even re-querying (e.g. $em->getRepository(UserObject::class)->findOneBy(['username' => 'XXXX']);) may not be enough because Doctrine is smart and tries to re-use things in memory whenever possible to avoid a fresh query.

Anyways, if this is NOT it, then you're right: something is saving this. But, as I mentioned, it "shouldn't" happen. It is however possible: if anything else in your system called $em->flush() after the object was modified (I can't think of why something might do this, but it's possible you have some custom listener or something somewhere) then it would cause any modified objects to be flushed. This is a known "quirk" of how Symfony's validator system works: the fact that you need to modify an object before you validate it means that if something accidentally calls flush later during the same request, then it will flush that object. In practice, I've never been bit by this - it would likely only be possible if you had an event listener on Symfony's ResponseEvent or ExceptionEvent that flushed something to the database.

Anyways, let me know what you find out!

Cheers!

Simon L. | weaverryan | posted 2 years ago | edited

Hi weaverryan !

I don't know what to say... You are such an expert... You were totally right about the test behaviour. If I refresh the object, it works well...

Again, thank you so much for your in-depth help and your great quality courses.

Have a great day :)

weaverryan | SFCASTS | Simon L. | posted 2 years ago | edited

Hey Simon L.!

Sweet! Glad we got it worked out! That test behavior is *tricky*, unfortunately!

Cheers!

Simon L. | posted 2 years ago | edited

Hi there !

Validators are great. But just to check if I understood everything well, could we achieve the same result using Voters and "security_post_denormalize" as described below:

In the Entity:
`
/**

@ApiResource(
collectionOperations={
"post" = {
"security_post_denormalize" = "is_granted('CHEESE_CREATE', object)",
},
},
`

(In my example, I do not want all users to be able to create a cheese_listing, only the one with CHEESE_CREATE role, but it could be that every user can have this role, it doesn't really change the logic I want to point out).

In the voter:
Some logic to check if (object->getOwner() === $user) OR ($user is admin)

By using Voters we can sort users with a specific role (eg. CHEESE_CREATE), but we could do that in Validators too (by checking $user->getRoles()).

Am I wrong ?

MolloKhan | SFCASTS | Simon L. | posted 2 years ago | edited

Hey Simon L.

Validating data and authorizing access are two different things. Validators are meant to, well, validate that your data is in a healthy/expected state, and authorizing is for doing things like you want

It's not recommended to grant access to some resource by checking a user's roles via the getRoles() method, you have to do it via the is_granted() function

I hope it makes sense to you. Cheers!

1 Reply

Simon L. | MolloKhan | posted 2 years ago | edited

Hi MolloKhan !

Thanks for your reply :)

julien_bonnier | posted 3 years ago

Hi there,

Just wondering, why isn't the throw statement on top of the validator's validate method?

sadikoff | SFCASTS | julien_bonnier | posted 3 years ago | edited

Hey julien_bonnier

Did you mean docblock above function definition?

Cheers!

Hannah R. | posted 3 years ago

Lets say i have a User with ROLE_EDITOR who is not the owner.
If this user is trying to edit the cheeselisting, without trying to set the owner field, there will be a violation.

Shouldnt there only be a validation check for $value->getId() !== $user->getId()) if the owner field is being set or updated?

MolloKhan | SFCASTS | Hannah R. | posted 3 years ago | edited

Hey Hannah R.

I think you will need to create a custom Voter for implementing such logic. In this chapter you can watch (or read) how to do it https://symfonycasts.com/sc...

Cheers!

Hannah R. | MolloKhan | posted 3 years ago | edited

Hey MolloKhan ,
just to make it clear: i have a voter that allows users with ROLE_EDITOR to edit a cheeseListing (as in the chapter you linked). But if any user with ROLE_EDITOR is trying to edit the cheeseListing, this Validator is preventing it. Because it checks for $value->getId() !== $user->getId()) even if the owner field is not being updated.

If ROLE_EDITOR tries to update the title only for example, it fails with "Cannot set owner to a different user". The user with ROLE_EDITOR didn't try to set the owner ... Its not even writebale with a put or patch because the owner field has this in the annotation:

@Groups({"cheeselisting:read", "cheeselisting:collection:post"})

MolloKhan | SFCASTS | Hannah R. | posted 3 years ago | edited

Hey Hannah R.

Got it now. The problem comes from the IsValidOwnerValidator, you need make it smarter. If the logged in User has the role ROLE_EDITOR, then, you'll have to check that the owner property didn't change. To do that, you'll need to get the initial version of your CheeseListing object. It's a bit tricky because of how Doctrine + Symfony Forms works but here I leave you a good article I found that explains how to do so https://medium.com/@ger86/symfony-validate-an-object-based-on-its-previous-version-4b6ca7a85dc6 - scroll down to the Defining a “Validator” class section

Cheers!

Alberto rafael P. | posted 3 years ago

Tengo muchas ganas de ver este tambien

Ad F. | Alberto rafael P. | posted 3 years ago

English Only :)

1 Reply

This tutorial works great for Symfony 5 and API Platform 2.5/2.6.

Symfony 4 API Platform 2.4

What PHP libraries does this tutorial use?

// composer.json
{
    "require": {
        "php": "^7.1.3, <8.0",
        "ext-ctype": "*",
        "ext-iconv": "*",
        "api-platform/core": "^2.1", // v2.4.5
        "composer/package-versions-deprecated": "^1.11", // 1.11.99
        "doctrine/annotations": "^1.0", // 1.13.2
        "doctrine/doctrine-bundle": "^1.6", // 1.11.2
        "doctrine/doctrine-migrations-bundle": "^2.0", // v2.0.0
        "doctrine/orm": "^2.4.5", // v2.7.2
        "nelmio/cors-bundle": "^1.5", // 1.5.6
        "nesbot/carbon": "^2.17", // 2.21.3
        "phpdocumentor/reflection-docblock": "^3.0 || ^4.0", // 4.3.1
        "symfony/asset": "4.3.*", // v4.3.2
        "symfony/console": "4.3.*", // v4.3.2
        "symfony/dotenv": "4.3.*", // v4.3.2
        "symfony/expression-language": "4.3.*", // v4.3.2
        "symfony/flex": "^1.1", // v1.18.7
        "symfony/framework-bundle": "4.3.*", // v4.3.2
        "symfony/http-client": "4.3.*", // v4.3.3
        "symfony/monolog-bundle": "^3.4", // v3.4.0
        "symfony/security-bundle": "4.3.*", // v4.3.2
        "symfony/twig-bundle": "4.3.*", // v4.3.2
        "symfony/validator": "4.3.*", // v4.3.2
        "symfony/webpack-encore-bundle": "^1.6", // v1.6.2
        "symfony/yaml": "4.3.*" // v4.3.2
    },
    "require-dev": {
        "hautelook/alice-bundle": "^2.5", // 2.7.3
        "symfony/browser-kit": "4.3.*", // v4.3.3
        "symfony/css-selector": "4.3.*", // v4.3.3
        "symfony/maker-bundle": "^1.11", // v1.12.0
        "symfony/phpunit-bridge": "^4.3", // v4.3.3
        "symfony/stopwatch": "4.3.*", // v4.3.2
        "symfony/web-profiler-bundle": "4.3.*" // v4.3.2
    }
}

Edit on GitHub

TRACK

Symfony 5 >

COURSE

API Platform Part 2: Security

Security Logic in the Validator

Keep on Learning!

Making sure the User Is Authenticated

Checking the Owner

Allowing Admins to do Anything

Requiring Owner

Leave a comment!

What PHP libraries does this tutorial use?

	... lines 1 - 9
	class IsValidOwnerValidator extends ConstraintValidator
	{
	private $security;

	public function __construct(Security $security)
	{
	$this->security = $security;
	}
	... lines 18 - 44
	}

	... lines 1 - 18
	public function validate($value, Constraint $constraint)
	{
	... lines 21 - 26
	$user = $this->security->getUser();
	if (!$user instanceof User) {
	... lines 29 - 32
	}
	... lines 34 - 43
	}
	... lines 45 - 46

	... lines 1 - 9
	class IsValidOwner extends Constraint
	{
	... lines 12 - 15
	public $message = 'Cannot set owner to a different user';
	... lines 17 - 18
	}

	... lines 1 - 9
	class IsValidOwnerValidator extends ConstraintValidator
	{
	... lines 12 - 18
	public function validate($value, Constraint $constraint)
	{
	... lines 21 - 27
	if (!$user instanceof User) {
	$this->context->buildViolation($constraint->anonymousMessage)
	->addViolation();

	return;
	}
	... lines 34 - 43
	}
	}

	... lines 1 - 18
	public function validate($value, Constraint $constraint)
	{
	... lines 21 - 34
	if (!$value instanceof User) {
	throw new \InvalidArgumentException('@IsValidOwner constraint must be put on a property containing a User object');
	}
	... lines 38 - 43
	}
	... lines 45 - 46

	... lines 1 - 18
	public function validate($value, Constraint $constraint)
	{
	... lines 21 - 39
	if ($value->getId() !== $user->getId()) {
	$this->context->buildViolation($constraint->message)
	->addViolation();
	}
	}
	... lines 45 - 46

	... lines 1 - 9
	class CheeseListingResourceTest extends CustomApiTestCase
	{
	... lines 12 - 13
	public function testCreateCheeseListing()
	{
	... lines 16 - 24
	$cheesyData = [
	'title' => 'Mystery cheese... kinda green',
	'description' => 'What mysteries does it hold?',
	'price' => 5000
	];
	... lines 30 - 44
	}
	... lines 46 - 74
	}

	... lines 1 - 13
	public function testCreateCheeseListing()
	{
	... lines 16 - 30
	$client->request('POST', '/api/cheeses', [
	'json' => $cheesyData,
	]);
	... lines 34 - 44
	}
	... lines 46 - 76

	... lines 1 - 48
	class CheeseListing
	{
	... lines 51 - 95
	/**
	... lines 97 - 100
	* @Assert\NotBlank()
	*/
	private $owner;
	... lines 104 - 207
	}