Security Logic in the Validator

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

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

Login Subscribe

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.

... 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.

... 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

... 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.

... 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

... 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!

... 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.

... 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.

... 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.

... 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().

... 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!

  • 2020-04-27 Vladimir Sadicov

    Hey Julien Bonnier

    Did you mean docblock above function definition?

    Cheers!

  • 2020-04-26 Julien Bonnier

    Hi there,

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

  • 2020-04-22 Diego Aguiar

    Hey Hannah

    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/s... - scroll down to the Defining a “Validator” class section

    Cheers!

  • 2020-04-22 Hannah

    Hey Diego Aguiar ,
    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"})

  • 2020-04-22 Diego Aguiar

    Hey Hannah

    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!

  • 2020-04-20 Hannah

    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?

  • 2020-01-05 cybernet2u

    English Only :)

  • 2019-09-12 Alberto

    Tengo muchas ganas de ver este tambien