From de320f81245bec2d211daa1fd61721230eaa1eb1 Mon Sep 17 00:00:00 2001 From: "Emma C. Hughes" <84008144+emmachughes@users.noreply.github.com> Date: Tue, 14 Nov 2023 09:36:59 +0100 Subject: [PATCH] Use Laravel auth to check for features (#2613) * use authorization system to check for features * revery policy change * fix mistake & test * fix logout with login disabled * fix mistake --- sourcecode/hub/app/Http/Kernel.php | 3 - .../Http/Middleware/ShareFeaturesWithView.php | 30 -------- .../hub/app/Http/Middleware/ToggleFeature.php | 29 -------- .../hub/app/Providers/AuthServiceProvider.php | 28 ++++++++ .../views/components/content-card.blade.php | 71 ++++++++++--------- .../components/navbar/navbar-bottom.blade.php | 22 +++--- .../components/navbar/navbar-top.blade.php | 22 +++--- .../hub/resources/views/login/index.blade.php | 4 +- sourcecode/hub/routes/web.php | 20 ++++-- sourcecode/hub/tests/Feature/UserTest.php | 8 +-- 10 files changed, 111 insertions(+), 126 deletions(-) delete mode 100644 sourcecode/hub/app/Http/Middleware/ShareFeaturesWithView.php delete mode 100644 sourcecode/hub/app/Http/Middleware/ToggleFeature.php diff --git a/sourcecode/hub/app/Http/Kernel.php b/sourcecode/hub/app/Http/Kernel.php index d6a65ae770..9dc3f6a472 100644 --- a/sourcecode/hub/app/Http/Kernel.php +++ b/sourcecode/hub/app/Http/Kernel.php @@ -39,13 +39,11 @@ class Kernel extends HttpKernel \App\Http\Middleware\LtiShareWithView::class, \App\Http\Middleware\VerifyCsrfToken::class, \Illuminate\Routing\Middleware\SubstituteBindings::class, - \App\Http\Middleware\ShareFeaturesWithView::class, \App\Http\Middleware\ContentSecurityPolicy::class, ], 'stateless' => [ \Illuminate\Routing\Middleware\SubstituteBindings::class, - \App\Http\Middleware\ShareFeaturesWithView::class, \App\Http\Middleware\ContentSecurityPolicy::class, ], @@ -68,7 +66,6 @@ class Kernel extends HttpKernel 'auth.session' => \Illuminate\Session\Middleware\AuthenticateSession::class, 'cache.headers' => \Illuminate\Http\Middleware\SetCacheHeaders::class, 'can' => \Illuminate\Auth\Middleware\Authorize::class, - 'feature' => \App\Http\Middleware\ToggleFeature::class, 'guest' => \App\Http\Middleware\RedirectIfAuthenticated::class, 'lti.launch-type' => \App\Http\Middleware\LtiLaunchType::class, 'password.confirm' => \Illuminate\Auth\Middleware\RequirePassword::class, diff --git a/sourcecode/hub/app/Http/Middleware/ShareFeaturesWithView.php b/sourcecode/hub/app/Http/Middleware/ShareFeaturesWithView.php deleted file mode 100644 index de9408d9ef..0000000000 --- a/sourcecode/hub/app/Http/Middleware/ShareFeaturesWithView.php +++ /dev/null @@ -1,30 +0,0 @@ -viewFactory->share('features', $this->features); - - return $next($request); - } -} diff --git a/sourcecode/hub/app/Http/Middleware/ToggleFeature.php b/sourcecode/hub/app/Http/Middleware/ToggleFeature.php deleted file mode 100644 index 5458d8d285..0000000000 --- a/sourcecode/hub/app/Http/Middleware/ToggleFeature.php +++ /dev/null @@ -1,29 +0,0 @@ -features->enabled($feature)) { - abort(Response::HTTP_NOT_FOUND, "The '$feature' feature is disabled"); - } - - return $next($request); - } -} diff --git a/sourcecode/hub/app/Providers/AuthServiceProvider.php b/sourcecode/hub/app/Providers/AuthServiceProvider.php index eb1bb7c580..d06696e6f1 100644 --- a/sourcecode/hub/app/Providers/AuthServiceProvider.php +++ b/sourcecode/hub/app/Providers/AuthServiceProvider.php @@ -4,6 +4,7 @@ namespace App\Providers; +use App\Configuration\Features; use App\Models\Content; use App\Models\LtiTool; use App\Models\User; @@ -12,6 +13,8 @@ use Illuminate\Foundation\Support\Providers\AuthServiceProvider as ServiceProvider; use Illuminate\Support\Facades\Gate; +use function request; + class AuthServiceProvider extends ServiceProvider { /** @@ -32,5 +35,30 @@ public function boot(): void Gate::define('admin', function (User $user) { return $user->admin ?? false; }); + + Gate::define('login', function (User|null $user) { + $request = request(); + + return !$request->hasPreviousSession() || !$request->session()->has('lti'); + }); + + Gate::define('register', function (User|null $user) { + $features = app()->make(Features::class); + + if (!$features->isSignupEnabled()) { + return false; + } + + $request = request(); + + return !$request->hasPreviousSession() || + !$request->session()->has('lti'); + }); + + Gate::define('reset-password', function (User|null $user) { + $features = app()->make(Features::class); + + return $features->isForgotPasswordEnabled(); + }); } } diff --git a/sourcecode/hub/resources/views/components/content-card.blade.php b/sourcecode/hub/resources/views/components/content-card.blade.php index 4287414267..985f5ad721 100755 --- a/sourcecode/hub/resources/views/components/content-card.blade.php +++ b/sourcecode/hub/resources/views/components/content-card.blade.php @@ -62,41 +62,48 @@ class="btn btn-secondary btn-sm d-none d-md-inline-block me-1" {{ trans('messages.edit-content') }} @endcan -
- - -
+ @canany(['view', 'edit', 'delete'], $content) +
+ + +
+ @endcan
{{ $views }}
- diff --git a/sourcecode/hub/resources/views/components/navbar/navbar-bottom.blade.php b/sourcecode/hub/resources/views/components/navbar/navbar-bottom.blade.php index b6cd4faea7..b659242b7b 100644 --- a/sourcecode/hub/resources/views/components/navbar/navbar-bottom.blade.php +++ b/sourcecode/hub/resources/views/components/navbar/navbar-bottom.blade.php @@ -90,16 +90,18 @@ class="dropdown-item" @else -
  • - - {{ trans('messages.log-in') }} - -
    • + @can('login') +
  • + + {{ trans('messages.log-in') }} + +
    • + @endcan - @if ($features->isSignupEnabled()) + @can('register')
    • - @endif + @endcan @endauth diff --git a/sourcecode/hub/resources/views/components/navbar/navbar-top.blade.php b/sourcecode/hub/resources/views/components/navbar/navbar-top.blade.php index 9df9d1d658..51536a8357 100644 --- a/sourcecode/hub/resources/views/components/navbar/navbar-top.blade.php +++ b/sourcecode/hub/resources/views/components/navbar/navbar-top.blade.php @@ -99,16 +99,18 @@ class="dropdown-item" @else -
  • - - {{ trans('messages.log-in') }} - -
    • + @can('login') +
  • + + {{ trans('messages.log-in') }} + +
    • + @endcan - @if ($features->isSignupEnabled()) + @can('register')
    • - @endif + @endcan @endauth diff --git a/sourcecode/hub/resources/views/login/index.blade.php b/sourcecode/hub/resources/views/login/index.blade.php index 33dcc648c1..1b5fba728e 100644 --- a/sourcecode/hub/resources/views/login/index.blade.php +++ b/sourcecode/hub/resources/views/login/index.blade.php @@ -20,11 +20,11 @@ {{ trans('messages.log-in') }} - @if ($features->isForgotPasswordEnabled()) + @can('reset-password') {{ trans('messages.forgot-password') }} - @endif + @endcan diff --git a/sourcecode/hub/routes/web.php b/sourcecode/hub/routes/web.php index 3124b26000..14fa17d7ab 100644 --- a/sourcecode/hub/routes/web.php +++ b/sourcecode/hub/routes/web.php @@ -31,12 +31,20 @@ return view('welcome'); })->name('home'); -Route::controller(LoginController::class)->group(function () { - Route::get('/login', 'login')->name('login'); - Route::post('/login', 'check')->name('login_check'); - Route::post('/log-out', 'logout')->name('log_out'); +Route::middleware('can:login')->group(function () { + Route::get('/login') + ->uses([LoginController::class, 'login']) + ->name('login'); + + Route::post('/login') + ->uses([LoginController::class, 'check']) + ->name('login_check'); }); +Route::post('/log-out') + ->uses([LoginController::class, 'logout']) + ->name('log_out'); + Route::controller(ContentController::class)->group(function () { Route::get('/content', 'index')->name('content.index'); @@ -103,12 +111,12 @@ }); Route::controller(UserController::class)->group(function () { - Route::middleware('feature:sign-up')->group(function () { + Route::middleware('can:register')->group(function () { Route::get('/register', 'register')->name('register'); Route::post('/register', 'store'); }); - Route::middleware('feature:forgot-password')->group(function () { + Route::middleware('can:reset-password')->group(function () { Route::get('/forgot-password', 'showForgotPasswordForm')->name('forgot-password'); Route::post('/forgot-password', 'sendResetLink')->name('forgot-password-send'); diff --git a/sourcecode/hub/tests/Feature/UserTest.php b/sourcecode/hub/tests/Feature/UserTest.php index 1ced3b28d2..a83994379a 100644 --- a/sourcecode/hub/tests/Feature/UserTest.php +++ b/sourcecode/hub/tests/Feature/UserTest.php @@ -24,9 +24,9 @@ public function testSignupsAreUsuallyEnabled(): void public function testSignupCanBeDisabled(): void { - config(['features.sign-up' => false]); + config()->set('features.sign-up', false); - $this->get('/register')->assertNotFound(); + $this->get('/register')->assertForbidden(); } public function testForgotPasswordIsUsuallyEnabled(): void @@ -36,9 +36,9 @@ public function testForgotPasswordIsUsuallyEnabled(): void public function testForgotPasswordCanBeDisabled(): void { - config(['features.forgot-password' => false]); + config()->set('features.forgot-password', false); - $this->get('/forgot-password')->assertNotFound(); + $this->get('/forgot-password')->assertForbidden(); } public function testSerialisation(): void