Securing Our User Endpoint - Part 1
In this video we will be learning how restrict our /users endpoint, stopping a logged in User from accessing any other User data than their own data.
We will restricting the endpoint in such a way that if a User were to visit their own ID, that would be fine. But if a User visits either another, valid ID, or a made up User ID, they would see a 403 unauthorised error.
This way, a malicious User cannot determine whether a resource is valid, purely by the return code. This is a little overkill, in this situation, however for the amount of effort involved in doing this (not much), we might as well get this extra functionality.
The Behat tests to prove these situations are as follows:
Scenario: User cannot GET a different User's personal data
When I send a "GET" request to "/users/u2"
Then the response code should 403
Scenario: User cannot determine if another User ID is active
When I send a "GET" request to "/users/u100"
Then the response code should 403I follow a set pattern when creating my endpoints using FOSRESTBundle. This is the same pattern I followed during my FOSRESTBundle tutorial course, and it continues to serve me well to this day.
The way I do this is to have my controller implement the standard REST verbs - GET, POST, PUT, DELETE, etc - but hand over all the grunt work to a Handler of some sort.
The Handler will contain all the standard REST verbs, but with the inclusion of an all method also, which is the second form of GET.
To clarify what I mean here, the standard routing output for an endpoint might look like this:
cget_accounts GET ANY ANY /accounts
get_accounts GET ANY ANY /accounts/{accountId}
post_accounts POST ANY ANY /accounts
patch_accounts PATCH ANY ANY /accounts/{accountId}
put_accounts PUT ANY ANY /accounts/{accountId}
delete_accounts DELETE ANY ANY /accounts/{accountId}Disregarding the details here, the first two are most interesting to us at this stage. Both are implementations of GET. I won't go into the exact specifics here, but if unsure, be sure to check out this video and this video, which cover this further.
I don't much like the name cget, so instead I call this all in my domain. It's still not the best name, but it makes sense to me. Hopefully this explains why I have a get and an all.
Here is the standard interface I use:
<?php
// /src/AppBundle/Handler/HandlerInterface.php
namespace AppBundle\Handler;
/**
* Interface HandlerInterface
* @package AppBundle\Handler
*/
interface HandlerInterface
{
/**
* @param int $id
* @return mixed
*/
public function get($id);
/**
* @param int $limit
* @param int $offset
* @return mixed
*/
public function all($limit, $offset);
/**
* @param array $parameters
* @param array $options
* @return mixed
*/
public function post(array $parameters, array $options);
/**
* @param mixed $resource
* @param array $parameters
* @param array $options
* @return mixed
*/
public function put($resource, array $parameters, array $options);
/**
* @param mixed $resource
* @param array $parameters
* @param array $options
* @return mixed
*/
public function patch($resource, array $parameters, array $options);
/**
* @param mixed $resource
* @return mixed
*/
public function delete($resource);
}The docblocks need a bit of tidying in truth, but that's for another time.
This interface is going to be implemented by any handler that we create - a UserHandler, an AccountHandler, and so on.
That said, not every method will make sense in the context of the given endpoint. Our Behat User feature says the following:
Scenario: User cannot DELETE their personal data
When I send a "DELETE" request to "/users/u1"
Then the response code should 405Yet, our UserHandler will still have to implement the get method, thanks to the interface. We don't want our Users to be able to delete their own data. So what should we do here?
Well, I simply throw a \DomainException, if a developer (me!) tries to do this.
The nice thing is we could maybe have one UserHandler implementation for regular Users, and another UserHandler implementation for administrators, and maybe the admin implementation could delete Users. Who knows? It leaves options open for us to composer our code a little better.
You may be wondering why we don't have an AdministrationUserHandler and a StandardUserHandler or similar. That's because the restriction doesn't really belong with the handler. In my opinion, it belongs with the repository.
Repository of Confusion
Even though our controllers delegate the responsibility for the action to the handler, the handler also delegates responsibility as much as possible.
The handler asks the repository for the data it needs.
In this instance, I refer not to the typical Doctrine repository, but rather to the concept of a repository in the sense of a domain driven design type architecture. I have to say, I don't think this is DDD in the purest sense of the term, so this shouldn't be viewed as such.
I use the term repository to refer to a collection of entities that live somewhere. This might be in an array, it might be in our database (it will be, in our case), or somewhere else entirely. Where the entities come from really is of no concern to the handler. They just appear as requested.
The handler will have something (the dependency) implementing the required repository interface injected during construction. This is nothing unusual - it's simply how we've always created and used Symfony services.
Which ultimately makes our Handler get logic really trivial:
public function get($id)
{
return $this->repository->find($id);
}Again, you can see here how the Handler doesn't care about whether this request will succeed or fail, whether the requesting User has access to this $id or not. It just delegates responsibility and gets the heck out of the way.
Testing our UserHandler with PHPSpec
We are building this RESTful API using Behat for our integration tests, and PHPSpec for our lower level / unit tests.
Behat will cover whether this code generally behaves as expected. But we can prove the code works at a function level by writing tests for each method with PHPSpec.
<?php
// /spec/AppBundle/Handler/UserHandlerSpec.php
namespace spec\AppBundle\Handler;
use AppBundle\Repository\UserRepositoryInterface;
use PhpSpec\ObjectBehavior;
use Prophecy\Argument;
class UserHandlerSpec extends ObjectBehavior
{
private $repository;
function let(UserRepositoryInterface $repository)
{
$this->repository = $repository;
$this->beConstructedWith($repository);
}
function it_is_initializable()
{
$this->shouldHaveType('AppBundle\Handler\UserHandler');
$this->shouldImplement('AppBundle\Handler\HandlerInterface');
}
function it_can_GET()
{
$id = 777;
$this->get($id);
$this->repository->find($id)->shouldHaveBeenCalled();
}
function it_cannot_get_ALL()
{
$this->shouldThrow('\DomainException')->during('all', [1,2]);
}
function it_cannot_POST()
{
$this->shouldThrow('\DomainException')->during('post', [['param1']]);
}
function it_cannot_PUT()
{
$this->shouldThrow('\DomainException')->during('put', [['param1'], []]);
}
function it_should_allow_PATCH(User $user)
{
// to be implemented
}
function it_should_throw_if_PATCH_not_given_a_valid_user()
{
// to be implemented
}
function it_cannot_DELETE()
{
$this->shouldThrow('\DomainException')->during('delete', ['something']);
}
}As already discussed, we cannot POST, PUT, or DELETE in the concept of our domain, so these will throw \DomainExceptions.
We haven't actually created UserRepositoryInterface as of yet, so this test would currently fail. But it does show the requirements
Our UserRepositoryInterface will need a find method at this stage, and that's about it. In the scope of our PHPSpec test, we don't care at all about what the real result of the find method would be, only that it has been called with the expected $id.
Why Such A Specific Repository Interface?
As a side note, you may be wondering why we have a UserRepositoryInterface - which would imply we will need an AccountRepositoryInterface, and a FileRepositoryInterface as our API grows.
Yes, unfortunately we will.
The problem with a generic interface would only become evident as our implementation evolved. Having taken this architecture into production, I did originally start with a generic repository interface, but hit upon a problem.
Let's quickly review a future version of this interface to see why:
<?php
namespace AppBundle\Repository;
use AppBundle\Model\UserInterface;
/**
* Interface UserRepositoryInterface
* @package AppBundle\Repository
*/
interface UserRepositoryInterface
{
/**
* @param UserInterface $user
* @param array $arguments
*/
public function save(UserInterface $user, array $arguments = ['flush'=>true]);
/**
* @param UserInterface $user
* @param array $arguments
*/
public function delete(UserInterface $user, array $arguments = ['flush'=>true]);
/**
* @param $id
* @return mixed|null
*/
public function find($id);
}As mentioned earlier, we may want an different version, or versions of this repository that may not be so restrictive.
In PHP, we can't have an interface that accepts a generic type - yet - so aside from making a wholesale change to the underlying structure of the application (maybe having anything saveable, or deletable implementing a set interface), then a compromise had to be made... which led to separate interfaces. Not great, but not the worst thing either.
Creating the UserRepositoryInterface
Now we know the required method(s) on our UserRepositoryInterface, we can go ahead and create one or more implementations of the interface and proceed with making our handler actually useful.
In the next video we will create two variants of the UserRepositoryInterface.
