Why do we need to check if user is instance of UserInterface

Question

I've noticed in FOSUserBundle Controllers (ProfileController) the check if the $user is instance of UserInteface

$user = $this->getUser();
if (!is_object($user) || !$user instanceof UserInterface) {
    throw new AccessDeniedException('This user does not have access to this section.');
}

Is it enough to check only if (!is_object($user))?

If my user entity extends FOS\UserBundle\Model\User, in which case $user will not be instance of UserInterface?

Answer 1

Yes if your code isn't meant be open-sourced otherwise no.

Not checking for the object's instance doesn't insure that the object returned by the method getUser() will have all the methods you are expecting (example: getUsername()).

If you look at the getUser() method from Controller.php it does not necessarily return a user object. In fact, you could set up Symfony2 firewall in a way that getUser() would return a different object of a different instance.

Admitting that we have an interface UserInterface which defines getUsername().

In the following code, our User object does not implement UserInterface.

$user = $this->getUser();
if (!is_object($user)) {
    $user->getUsername();
}

This code will throw an error because getUsername() does not exist on the object, instead the code should have been the following:

$user = $this->getUser();
if (!is_object($user) || !$user instanceof UserInterface) {
    $user->getUsername();
}

If the user object does not implement the proper interface then the code won't error out because it won't be executed.

Avoid checking the object like the following

$user = $this->getUser();
if (!is_object($user) || !$user instanceof User) {
    $user->getRoles();
}

If someone extends the User object then the if statement won't be executed anymore because $user will not be an instance of User but say ExtendedUser even though it has all the methods you need.

The other advantage of using interfaces is that you can implement multiple interfaces on a object.

class A implements C {}

class B extends A implements C, D {}

interface C {}

interface D {}

$nA = new A();
$nB = new B();

$nA instanceof A; // true - instance of A
$nA instanceof B; // false - pretty obvious, no relationship with B
$nA instanceof C; // true - A implements C
$nA instanceof D; // false - A does not implement D

$nB instanceof A; // false - B is not an instance of A
$nB instanceof B; // true - instance of B
$nB instanceof C; // true - A implements C, that's the key:
                  //        both A and B implements C but B is not an
                  //        instance of A.
$nB instanceof D; // true - A implements D

TLDR; interfaces are a great way to set expectations and avoid major headaches.

When you read through the code you can quickly identify the type of objects that are passed around. If someone changes the code it will either show a meaningful error or it will degrade gracefully (in this case, the user will be denied access).