Route protection with custom middleware in Laravel 8

Question

I am working on a blogging application in Laravel 8 .

The application assigns users roles and permissions. There is a many-to-many relationship between roles and permissions.

I have created a custom middleware to give users access to routes based on their permissions:

namespace App\Http\Middleware;
use Closure;
use Illuminate\Http\Request;
use Illuminate\Support\Facades\Auth;
class CheckUserPermissions
{
 /**
 * Handle an incoming request.
 *
 * @param \Illuminate\Http\Request $request
 * @param \Closure(\Illuminate\Http\Request): (\Illuminate\Http\Response|\Illuminate\Http\RedirectResponse) $next
 * @return \Illuminate\Http\Response|\Illuminate\Http\RedirectResponse
 */
 // Permissions checker
 public function hasPermissionTo($permission) {
 return in_array($permission, Auth::user()->role->permissions->pluck('slug')->toArray());
 }
 public function handle(Request $request, Closure $next, ...$permissions)
 {
 // Check user permissions
 foreach ($permissions as $permission) {
 if (!$this->hasPermissionTo($permission)) { 
 $permission_label = join(' ', explode('-', $permission));
 return redirect()->back()->with('error', 'You do not have permission to ' . $permission_label);
 }
 }
 return $next($request);
 }
}

In routes\web.php I have:

// Dashboard routes
Route::group(['prefix' => 'dashboard', 'middleware' => ['auth']], function() {
 Route::get('/', [DashboardController::class, 'index'])->name('dashboard');
 // Settings routes
 Route::group(['prefix' => 'settings', 'middleware' => ['checkUserPermissions:edit-settings']], function() {
 Route::get('/', [SettingsController::class, 'index'])->name('dashboard.settings');
 Route::post('/update', [SettingsController::class, 'update'])->name('dashboard.settings.update');
 });
 // Pages routes
 Route::group(['prefix' => 'pages'], function() {
 Route::get('/', [PageController::class, 'index'])->name('dashboard.pages')->middleware('checkUserPermissions:view-pages');
 Route::get('/new', [PageController::class, 'create'])->name('dashboard.pages.new')->middleware('checkUserPermissions:add-pages');
 Route::post('/add', [PageController::class, 'save'])->name('dashboard.pages.add');
 Route::get('/edit/{id}', [PageController::class, 'edit'])->name('dashboard.pages.edit')->middleware('checkUserPermissions:edit-pages');
 Route::post('/update/{id}', [PageController::class, 'update'])->name('dashboard.pages.update');
 Route::get('/delete/{id}', [PageController::class, 'delete'])->name('dashboard.pages.delete')->middleware('checkUserPermissions:delete-pages');
 });
 // Category routes
 Route::group(['prefix' => 'categories'], function() {
 Route::get('/', [ArticleCategoryController::class, 'index'])->name('dashboard.categories')->middleware('checkUserPermissions:view-categories');
 Route::get('/new', [ArticleCategoryController::class, 'create'])->name('dashboard.categories.new')->middleware('checkUserPermissions:add-categories');
 Route::post('/add', [ArticleCategoryController::class, 'save'])->name('dashboard.categories.add');
 Route::get('/edit/{id}', [ArticleCategoryController::class, 'edit'])->name('dashboard.categories.edit')->middleware('checkUserPermissions:edit-categories');
 Route::post('/update/{id}', [ArticleCategoryController::class, 'update'])->name('dashboard.categories.update');
 Route::get('/delete/{id}', [ArticleCategoryController::class, 'delete'])->name('dashboard.categories.delete')->middleware('checkUserPermissions:delete-categories');
 });
 // Article routes
 Route::group(['prefix' => 'articles'], function() {
 Route::get('/', [ArticleController::class, 'index'])->name('dashboard.articles')->middleware('checkUserPermissions:view-articles');
 Route::get('/new', [ArticleController::class, 'create'])->name('dashboard.articles.new')->middleware('checkUserPermissions:add-articles');
 Route::post('/add', [ArticleController::class, 'save'])->name('dashboard.articles.add');
 Route::get('/edit/{id}', [ArticleController::class, 'edit'])->name('dashboard.articles.edit')->middleware('checkUserPermissions:edit-articles');
 Route::post('/update/{id}', [ArticleController::class, 'update'])->name('dashboard.articles.update');
 Route::get('/delete/{id}', [ArticleController::class, 'delete'])->name('dashboard.articles.delete')->middleware('checkUserPermissions:delete-articles');
 });
 // Comments routes
 Route::group(['prefix' => 'comments'], function() {
 Route::get('/', [CommentController::class, 'index'])->name('dashboard.comments')->middleware('checkUserPermissions:view-comments');
 Route::get('/delete/{id}', [CommentController::class, 'delete'])->name('dashboard.comments.delete')->middleware('checkUserPermissions:delete-comments');
 Route::get('/approve/{id}', [CommentController::class, 'approve'])->name('dashboard.comments.approve')->middleware('checkUserPermissions:approve-comments');
 Route::get('/unapprove/{id}', [CommentController::class, 'unapprove'])->name('dashboard.comments.unapprove')->middleware('checkUserPermissions:unapprove-comments');
 });
 // User management routes
 Route::group(['prefix' => 'users'], function() {
 Route::get('/rights', [UserRightsController::class, 'index'])->name('user-rights')->middleware('checkUserPermissions:manage-user-rights');;
 Route::get('/rights/change-role/{id}', [UserRightsController::class, 'change_role'])->name('change-role')->middleware('checkUserPermissions:assign-user-roles');
 Route::post('/rights/update-role/{id}', [UserRightsController::class, 'update_role'])->name('update-role');
 Route::get('/rights/ban/{id}', [UserRightsController::class, 'ban_user'])->name('ban-user')->middleware('checkUserPermissions:ban-users');
 Route::get('/rights/activate/{id}', [UserRightsController::class, 'activate_user'])->name('activate-user')->middleware('checkUserPermissions:activate-users');
 });
});

The Super-admin has several abilities related to user management:

use App\Models\User;
use App\Models\Role;
class UserRightsController extends Controller
{
 public function roles() {
 return Role::all();
 }
 
 public function index() {
 $users = User::paginate(10);
 return view('dashboard/user-rights', ['users' => $users]);
 }
 public function change_role($id) {
 $user = User::find($id);
 return view('dashboard/change-role',['user' => $user, 'roles' => $this->roles()]);
 }
 public function update_role(Request $request, $id) {
 $user = User::find($id);
 $user->role_id = $request->get('role_id');
 $user->save();
 return redirect()->route('user-rights')->with('success', 'The role for ' . $user->first_name . ' ' . $user->last_name . ' was updated');
 }
 public function ban_user($id){
 User::find($id)->update(['active' => 0]);
 return redirect()->back()->with('success', 'The user is now banned');
 }
 public function activate_user($id){
 User::find($id)->update(['active' => 1]);
 return redirect()->back()->with('success', 'The user is now active');
 }
}

Questions:

1. Do you see any security issues in the code above?
2. Are there ways to make it more DRY?
3. Do you see any architecture flaw?

Answer 1

1. I see an avoidable single-use variable. And explode-join can be replaced with str_replace.

   $permission_label = join(' ', explode('-', $permission));
   return redirect()->back()->with('error', 'You do not have permission to ' . $permission_label);
   

   Can be reduced to:

   return redirect()->back()->with(
    'error',
    'You do not have permission to ' . str_replace('-', ' ', $permission)
   );
   

2. I'd probably not declare $users in index() nor $user in change_role().

3. I recommend adding type declarations to all of the $id arguments in the UserRightsController() class as well as $permission in the hasPermissionTo() method. Even handle() can have string ...$permissions.

I do not have any insights regarding security; I have no experience with Laravel.

Answer 2

I would add a lot of line breaks for readability

For example here:

public function hasPermissionTo($permission) {
 return in_array($permission, 
 Auth::user()
 ->role
 ->permissions
 ->pluck('slug')
 ->toArray()
 );
}

or here

public function update_role(Request $request, $id) {
 $user = User::find($id);
 $user->role_id = $request->get('role_id');
 $user->save();
 return redirect()
 ->route('user-rights')
 ->with('success', 'The role for ' 
 . $user->first_name . ' ' 
 . $user->last_name . ' was updated');
}

and in the web.php

Route::get('/unapprove/{id}', [CommentController::class, 'unapprove'])
 ->name('dashboard.comments.unapprove')
 ->middleware('checkUserPermissions:unapprove-comments');