Checking An Edge Case With Symfony Voters

I love Symfony Voters. The Access Control List is, in the vast majority of cases, totally overkill for my security needs. Symfony’s Security Voters are a very quick and easy alternative.

Yesterday I got caught out by an edge case I had not been testing for.

A little background on this – I have been creating a RESTful API using Symfony 3, Behat 3, PHPSpec, and a few other interesting technologies. If you are interested in knowing a little more about this then be sure to check out the video tutorial series here on CodeReviewVideos where I have been documenting as I go.

The video tutorial series covers three different entities – Users (FOSUserBundle), Accounts, and Files (with FlySystem).

I want to ensure the currently logged in User has access to the resources they are requesting. I’ve been doing this with Voters.

It’s likely easier to follow along with these words if I give them more context. Here’s the code for the File Voter:

And the implementation:

This all works and is passing.

However, there is an edge case I hadn’t considered.

What happens if $requestedFile->belongsToUser($loggedInUser) fails, or returns null? Well, bad times as it happens.

Why might it fail? Well, let’s look at that interface, and the real method implementation:

And the implementation:

Due to a mistake in my fixtures, the File wasn’t correctly being added to the Account I expected, so the call of getAccounts() was returning null.

In this case, I first fixed my fixtures as that situation should never really happen.

But what if it does? I don’t want to be throwing errors to the user. It’s easier to deny them access, log the fault (using monolog, and/or to ELK), and fix it in a slightly more controlled manner.

I added the following two specs to my FileVoterSpec test file:

And then changed up the implementation (uglify!):

Note the try/catch block. It’s definitely made the code uglier, but it ‘fixes’ a potential problem I hadn’t considered.

I say ‘fixes’ as this should never happen. But if I have learned anything over the last 15-odd years, it’s that those things that should never happen… they happen.

Shares

Published by

Code Review

Code Review

CodeReviewVideos is a video training site helping software developers learn Symfony faster and easier.

Leave a Reply

Your email address will not be published. Required fields are marked *