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
 );

• kbck: Thank you. I can't return boolean values directly because the code below the first return will be unreachable. However I will think about refactoring the buildPermissionName method.