Laravel user permission system

Question

I wanted to create a simple and easy to use permissions system. The ones I've found on the Internet would need to attach every permission to a route manually (e.g. Entrust).

My idea is to build a permission name out of the current action, so for example method 'store' in 'TestController' becomes 'test.store' permission.

Is it a good approach? I couldn't find anything similiar on the web.

class SecuredController extends Controller {

    /**
     * @var array
     * unguarded actions (method names)
     */
    protected $unguarded = [];

    /**
     * @var array
     * actions (method names) that need csrf token checked
     */
    protected $csrfguarded = [];

    private $permission;

    public function getCurrentPermission() {
        return $this->permission;
    }

    public function callAction($method, $parameters) {
        if(! $this->canAccessCurrentAction()) {
            return $this->accessDenied();
        }

        return parent::callAction($method, $parameters);
    }

    /**
     * @param string $action e.g. TestController@store
     *
     * @return bool
     * @throws \Illuminate\Session\TokenMismatchException
     */
    public function canAccessAction($action) {
        /**
         * split $action(e.g. TestController@store) into two parts
         * and build a permission name (e.g. test.store) and check for access
         */

        list($controller, $method) = explode('@', $action);

        if(in_array($method, $this->unguarded, true)) {
            return true;
        }

        if(in_array($method, $this->csrfguarded, true)
           && Session::token() !== Input::get('_token'))
        {
            throw new Illuminate\Session\TokenMismatchException;
        }

        if(! Auth::check()) {
            return false;
        }

        $this->permission = $this->buildPermissionName($controller, $method);

        if(Auth::getUser()->capable($this->permission)) {
            return true;
        }

        return false;
    }

    public function canAccessCurrentAction() {
        return $this->canAccessAction( \Route::getCurrentRoute()->getActionName());
    }

    public function buildPermissionName($controller, $method) {
        return strtolower(
            substr(
                $controller,
                strrpos($controller, '\\'),
                - 10 // 'controller' word
            ) . '.' . $method
        );
    }

    /**
     * Override it!
     */
    public function accessDenied() {
        if(Request::ajax()) {
            return Response::make('You cannot access this!', 403);
        }

        return Response::make('You cannot access this!', 403);
    }
}

The capable method of User:

public function capable($neededPermission) {
    foreach($this->getPermissions() as $permission) {
        if(substr($permission, -1) === '*') {
            $permission = substr($permission, 0, -1);

            if(startsWith($neededPermission, $permission)) {
                return true;
            }
        }
        elseif($permission === $neededPermission) {
            return true;
        }
    }

    return false;
}

Answer 1

I see a lot of if-else statements that could be improved upon.

Ones like these for example, are returning a boolean variable depending on an if-else statement with nothing else inside.

In those cases you can return boolean values directly.

  if(in_array($method, $this->unguarded, true)) {
      return true;
  }

into:

return (in_array($method, $this-unguarded, true));

---

The following code does the same thing for both 'cases', meaning that no matter what 'Request::ajax()' is, the same result will be given:

public function accessDenied() {
    if(Request::ajax()) {
        return Response::make('You cannot access this!', 403);
    }

    return Response::make('You cannot access this!', 403);
}

Which could simply be turned into:

public function accessDenied() {
    return Response::make('You cannot access this!', 403);
}

---

Your usage of the following code really bothers me, and I believe it could do with refactoring:

return strtolower(
        substr(
            $controller,
            strrpos($controller, '\\'),
            - 10 // 'controller' word
        ) . '.' . $method
    );