2

I have a great number of PHP classes which perform the business logic of my application, e.g.

class FileMaker{
  public function makeFile(){
    //some code here to make a file
  }
}

Now I need to add permissions checks to them. I want to avoid having to copy/paste $this->checkPermissions() or similar into every method in every class. What's the best way of achieving this?

I was thinking some kind of decorator pattern which could add permissions checks, e.g:

//decorator that can be used to check permissions
class PermissionsDecorator{
    private $ob;

    public function __construct($ob){
        $this->ob = $ob;
    }

    //magic method which is called when any function is called
    public function __call($name,$arguments){
        if($this->hasPermission(get_class($this->ob), $name)){
            call_user_func_array(array($this->ob,$name),$arguments);
        }else{
            throw new Exception('You do not have permission to perform this action.');
        }
    }

    public function hasPermission($class,$method){
        echo 'does the user have permission on '.$class.'->'.$method.'?<br/>';

        //do some logic here to work out whether the user has permission
        return true;
    }
}

The decorator above could apply to all classes because it uses the __call magic method, rather than specifying the exact methods to call. Then I could use code such as:

$fm = new PermissionsDecorator(new FileMaker());

This feels a bit 'hacky' to me though. Is there a standard way of doing this? Or will I just have to copy/paste permissions checks everywhere?

  • 1
    What should happen if a user wants to do an action that involves 3 or 4 business methods, but the user only has the right to perform the first two of them? – Bart van Ingen Schenau Feb 3 '15 at 10:56
  • @BartvanIngenSchenau In my application, the user can only ever call one 'permissionable' action at a time, so I don't think this will be a problem. – user1578653 Feb 3 '15 at 11:22
3

If the amount of work you would need to expend to refactor towards a transparent implementation of authentication in your controllers is indeed absolutely prohibitive, a decorator will work.

The reason it ought feel somewhat 'hacky' is because relying on magic methods makes functionality somewhat opaque (magic methods, as well as call_user_func also have some performance penalties, but they are usually negligable), and because decorators aren't best practice when what they add is something that is necessary for the decoratee to work as intended.

The most transparent, secure and flexible way to do this (if it's really the individual objects/methods that need to check the authentication, which I doubt - see below) would be to implement a strategy pattern. In the constructor of classes whose instances need to do authentication, you would require an object (e.g. DefaultUserAuthenticator) that meets the interface designed for the strategy (e.g. IUserAuthenticator). A reference to the concrete authenticator would be stored in each class (PHP automatically assigns objects by reference), and then, yes, you would have to call the authenticate method of the locally referenced authenticator everywhere you really need to authenticate.

But if you always immediately turn a request for execution of a certain function to the object and method that performs the function, and then have to check user permission in every single such method / object - it seems that this might be the actual problem.

When you handle the request and decide what to do with it, you ought to check permission as 'high up' in the decision-tree as possible, to avoid precisely the situation you find yourself in now. So you might have a front-controller / page-controller / helper-service which delegates to the specific objects only once the user has been authenticated.

If you are considering using a decorator-pattern, that means the method of authentication doesn't vary with each object/method. As such, you should be able to check permission once, then delegate either to an exception-handler or the object whose responsibility it is to actually perform the requested function.... the individual objects and methods shouldn't need to do this.

|improve this answer|||||
  • Thanks for your answer - your idea of having a front controller which checks the permissions is a good one! This avoids copying/pasting permissions checks everywhere. – user1578653 Feb 4 '15 at 10:20
  • I think this is a great answer because it points out the trade-off between flexibility and the added complexity (obscurity) of a design pattern. – Fuhrmanator Feb 5 '15 at 0:18

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.