From 1233f02d40b7d0b40edaddbe80be864c67af36e3 Mon Sep 17 00:00:00 2001 From: mwalbeck <magn3200@gmail.com> Date: Wed, 26 Oct 2016 13:10:30 +0200 Subject: [PATCH] Cleaned up admin and mod authorization on routes and added UserPolicy --- app/Http/Controllers/AdminController.php | 2 +- .../AdministrativeTestController.php | 2 +- .../AdministrativeUserController.php | 15 +++++- app/Http/Controllers/ModeratorController.php | 2 +- app/Http/Kernel.php | 1 - app/Http/Middleware/IsAdminOrMod.php | 24 ---------- app/Http/Middleware/IsAdministrator.php | 1 + app/Http/Middleware/IsModerator.php | 1 + app/Policies/UserPolicy.php | 45 ++++++++++++++++++ app/Providers/AuthServiceProvider.php | 1 + app/User.php | 22 +-------- resources/views/errors/403.blade.php | 47 +++++++++++++++++++ routes/web.php | 6 +-- 13 files changed, 116 insertions(+), 53 deletions(-) delete mode 100644 app/Http/Middleware/IsAdminOrMod.php create mode 100644 app/Policies/UserPolicy.php create mode 100644 resources/views/errors/403.blade.php diff --git a/app/Http/Controllers/AdminController.php b/app/Http/Controllers/AdminController.php index 37df4b5..bc9d1eb 100644 --- a/app/Http/Controllers/AdminController.php +++ b/app/Http/Controllers/AdminController.php @@ -13,7 +13,7 @@ class AdminController extends Controller { public function __construct() { - $this->middleware(['auth', 'is.admin']); + $this->middleware('auth'); } public function index() diff --git a/app/Http/Controllers/AdministrativeTestController.php b/app/Http/Controllers/AdministrativeTestController.php index 40d5a1a..f47ee95 100644 --- a/app/Http/Controllers/AdministrativeTestController.php +++ b/app/Http/Controllers/AdministrativeTestController.php @@ -16,7 +16,7 @@ class AdministrativeTestController extends Controller { public function __construct() { - $this->middleware(['auth', 'is.admin.mod']); + $this->middleware('auth'); } /** diff --git a/app/Http/Controllers/AdministrativeUserController.php b/app/Http/Controllers/AdministrativeUserController.php index be791ed..00bece4 100644 --- a/app/Http/Controllers/AdministrativeUserController.php +++ b/app/Http/Controllers/AdministrativeUserController.php @@ -13,7 +13,7 @@ class AdministrativeUserController extends Controller { public function __construct() { - $this->middleware(['auth', 'is.admin.mod']); + $this->middleware('auth'); } /** @@ -23,11 +23,15 @@ class AdministrativeUserController extends Controller */ public function confirmDeleteUser(User $user) { + $this->authorize('delete', $user); + return view('users.delete', compact('user')); } public function deleteUser(User $user) { + $this->authorize('delete', $user); + $user->deleteUser(); return redirect('/admin/users'); } @@ -40,6 +44,8 @@ class AdministrativeUserController extends Controller */ public function newUser() { + $this->authorize('create', User::class); + if (Auth::user()->isAdministrator()) { $groups = Group::all(); return view('users.new', compact('groups')); @@ -47,7 +53,6 @@ class AdministrativeUserController extends Controller return view('users.new'); } - /** * * Function for adding a user. @@ -55,6 +60,8 @@ class AdministrativeUserController extends Controller */ public function addUser(StoreUser $request) { + $this->authorize('create', User::class); + $user = new User; $user->addUser($request->all()); return redirect('/admin/users'); @@ -67,6 +74,8 @@ class AdministrativeUserController extends Controller */ public function editUser(User $user) { + $this->authorize('edit', $user); + if (Auth::user()->isAdministrator()) { $groups = Group::all(); return view("users.edit", compact("groups", "user")); @@ -76,6 +85,8 @@ class AdministrativeUserController extends Controller public function updateUser(User $user, StoreUser $request) { + $this->authorize('edit', $user); + $user->updateUser($request->all()); return redirect("/admin/users/group/$user->group_id"); } diff --git a/app/Http/Controllers/ModeratorController.php b/app/Http/Controllers/ModeratorController.php index ccfc173..c2b2c2c 100644 --- a/app/Http/Controllers/ModeratorController.php +++ b/app/Http/Controllers/ModeratorController.php @@ -10,7 +10,7 @@ class ModeratorController extends Controller { public function __construct() { - $this->middleware(['auth', 'is.mod']); + $this->middleware('auth'); } public function index() diff --git a/app/Http/Kernel.php b/app/Http/Kernel.php index d9ed4b5..b0fdf74 100644 --- a/app/Http/Kernel.php +++ b/app/Http/Kernel.php @@ -52,7 +52,6 @@ class Kernel extends HttpKernel 'can' => \Illuminate\Auth\Middleware\Authorize::class, 'guest' => \App\Http\Middleware\RedirectIfAuthenticated::class, 'throttle' => \Illuminate\Routing\Middleware\ThrottleRequests::class, - 'is.admin.mod' => \App\Http\Middleware\IsAdminOrMod::class, 'is.admin' => \App\Http\Middleware\IsAdministrator::class, 'is.mod' => \App\Http\Middleware\IsModerator::class, ]; diff --git a/app/Http/Middleware/IsAdminOrMod.php b/app/Http/Middleware/IsAdminOrMod.php deleted file mode 100644 index 5bb818e..0000000 --- a/app/Http/Middleware/IsAdminOrMod.php +++ /dev/null @@ -1,24 +0,0 @@ -<?php - -namespace App\Http\Middleware; - -use Closure; -use Illuminate\Support\Facades\Auth; - -class IsAdminOrMod -{ - /** - * Handle an incoming request. - * - * @param \Illuminate\Http\Request $request - * @param \Closure $next - * @return mixed - */ - public function handle($request, Closure $next) - { - if (Auth::user()->isAdministrator() || Auth::user()->isModerator()) { - return $next($request); - } - return redirect("/home"); - } -} diff --git a/app/Http/Middleware/IsAdministrator.php b/app/Http/Middleware/IsAdministrator.php index 5df8228..fcd4bb5 100644 --- a/app/Http/Middleware/IsAdministrator.php +++ b/app/Http/Middleware/IsAdministrator.php @@ -3,6 +3,7 @@ namespace App\Http\Middleware; use Closure; +use Illuminate\Support\Facades\Auth; class IsAdministrator { diff --git a/app/Http/Middleware/IsModerator.php b/app/Http/Middleware/IsModerator.php index ed90b3a..1b589b4 100644 --- a/app/Http/Middleware/IsModerator.php +++ b/app/Http/Middleware/IsModerator.php @@ -3,6 +3,7 @@ namespace App\Http\Middleware; use Closure; +use Illuminate\Support\Facades\Auth; class IsModerator { diff --git a/app/Policies/UserPolicy.php b/app/Policies/UserPolicy.php new file mode 100644 index 0000000..f9702a7 --- /dev/null +++ b/app/Policies/UserPolicy.php @@ -0,0 +1,45 @@ +<?php + +namespace App\Policies; + +use App\User; +use Illuminate\Auth\Access\HandlesAuthorization; + +class UserPolicy +{ + use HandlesAuthorization; + + /** + * Create a new policy instance. + * + * @return void + */ + public function __construct() + { + // + } + + public function create(User $user) + { + if ($user->isAdministrator() || $user->isModerator()) { + return true; + } + return false; + } + + public function update(User $user, User $user2) + { + if ($user->isAdministrator() || $user->isModerator() AND $user->group_id === $user2->group_id) { + return true; + } + return false; + } + + public function delete(User $user, User $user2) + { + if ($user->isAdministrator() || $user->isModerator() AND $user->group_id === $user2->group_id || $user === $user2) { + return true; + } + return false; + } +} diff --git a/app/Providers/AuthServiceProvider.php b/app/Providers/AuthServiceProvider.php index 996066d..c64bc38 100644 --- a/app/Providers/AuthServiceProvider.php +++ b/app/Providers/AuthServiceProvider.php @@ -14,6 +14,7 @@ class AuthServiceProvider extends ServiceProvider */ protected $policies = [ 'App\Model' => 'App\Policies\ModelPolicy', + 'App\User' => 'App\Policies\UserPolicy', ]; /** diff --git a/app/User.php b/app/User.php index d32981e..5fe1b89 100644 --- a/app/User.php +++ b/app/User.php @@ -111,28 +111,12 @@ class User extends Authenticatable /** * - * Function for deleting user. Checks if it is the user itself, - * a moderator in the same group or an administrator. - * If this is not case, the user will not be deleted. + * Function for deleting user. * */ public function deleteUser() { - if ($this->id == Auth::user()->id) { - $this->delete(); - return true; - } - - if (Auth::user()->group_id == $this->group_id && Auth::user()->isModerator) { - $this->delete(); - return true; - } - - if (Auth::user()->isAdministrator()) { - $this->delete(); - return true; - } - return false; + $this->delete(); } /** @@ -145,14 +129,12 @@ class User extends Authenticatable $email = trim($email); $this->email = $email; $this->update(); - return true; } public function updatePassword($password) { $this->passwordHash($password); $this->update(); - return true; } public function testTaken($test_id) diff --git a/resources/views/errors/403.blade.php b/resources/views/errors/403.blade.php new file mode 100644 index 0000000..eb76d26 --- /dev/null +++ b/resources/views/errors/403.blade.php @@ -0,0 +1,47 @@ +<!DOCTYPE html> +<html> + <head> + <title>Be right back.</title> + + <link href="https://fonts.googleapis.com/css?family=Lato:100" rel="stylesheet" type="text/css"> + + <style> + html, body { + height: 100%; + } + + body { + margin: 0; + padding: 0; + width: 100%; + color: #B0BEC5; + display: table; + font-weight: 100; + font-family: 'Lato', sans-serif; + } + + .container { + text-align: center; + display: table-cell; + vertical-align: middle; + } + + .content { + text-align: center; + display: inline-block; + } + + .title { + font-size: 72px; + margin-bottom: 40px; + } + </style> + </head> + <body> + <div class="container"> + <div class="content"> + <div class="title">Be right back.</div> + </div> + </div> + </body> +</html> diff --git a/routes/web.php b/routes/web.php index 9045299..51d17e0 100644 --- a/routes/web.php +++ b/routes/web.php @@ -36,7 +36,7 @@ Route::group(['prefix' => 'test'], function () { /*---------- Routes for the moderator section ----------*/ -Route::group(['prefix' => 'mod'], function () { +Route::group(['prefix' => 'mod', 'middleware' => 'is.mod'], function () { Route::get('/', 'ModeratorController@index'); Route::get('/tests', 'ModeratorController@showTests'); @@ -66,7 +66,7 @@ Route::group(['prefix' => 'mod'], function () { /*---------- Routes for the administrator section ----------*/ -Route::group(['prefix' => 'admin'], function () { +Route::group(['prefix' => 'admin', 'middleware' => 'is.admin'], function () { Route::get('/', 'AdminController@index'); @@ -85,7 +85,7 @@ Route::group(['prefix' => 'admin'], function () { Route::get('/questions/{question}/edit', 'AdministrativeTestController@editQuestion'); Route::patch('/questions/{question}', 'AdministrativeTestController@updateQuestion'); Route::get('/questions/{question}/delete', 'AdministrativeTestController@confirmDeleteQuestion'); - Route::delete('/questions/{question}/', 'AdministrativeTestController@deleteQuestion'); + Route::delete('/quphpestions/{question}/', 'AdministrativeTestController@deleteQuestion'); Route::get('/users', 'AdminController@showGroups'); Route::get('/users/all', 'AdminController@showAllUsers');