mirror of
https://github.com/BookStackApp/BookStack.git
synced 2025-04-01 21:05:13 +08:00
Started on some MFA access-time checks
Discovered some difficult edge cases: - User image loading in header bar when using local_secure storage - 404s showing user-specific visible content due to content listing on 404 page since user is in semi-logged in state. Maybe need to go through and change up how logins are handled to centralise and provide us better control at login time to prevent any auth level.
This commit is contained in:
parent
f696aa5eea
commit
78f9c01519
44
app/Auth/Access/Mfa/MfaSession.php
Normal file
44
app/Auth/Access/Mfa/MfaSession.php
Normal file
@ -0,0 +1,44 @@
|
||||
<?php
|
||||
|
||||
namespace BookStack\Auth\Access\Mfa;
|
||||
|
||||
class MfaSession
|
||||
{
|
||||
private const MFA_VERIFIED_SESSION_KEY = 'mfa-verification-passed';
|
||||
|
||||
/**
|
||||
* Check if MFA is required for the current user.
|
||||
*/
|
||||
public function requiredForCurrentUser(): bool
|
||||
{
|
||||
// TODO - Test both these cases
|
||||
return user()->mfaValues()->exists() || $this->currentUserRoleEnforcesMfa();
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if a role of the current user enforces MFA.
|
||||
*/
|
||||
protected function currentUserRoleEnforcesMfa(): bool
|
||||
{
|
||||
return user()->roles()
|
||||
->where('mfa_enforced', '=', true)
|
||||
->exists();
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if the current MFA session has already been verified.
|
||||
*/
|
||||
public function isVerified(): bool
|
||||
{
|
||||
return session()->get(self::MFA_VERIFIED_SESSION_KEY) === 'true';
|
||||
}
|
||||
|
||||
/**
|
||||
* Mark the current session as MFA-verified.
|
||||
*/
|
||||
public function markVerified(): void
|
||||
{
|
||||
session()->put(self::MFA_VERIFIED_SESSION_KEY, 'true');
|
||||
}
|
||||
|
||||
}
|
@ -37,4 +37,18 @@ class MfaController extends Controller
|
||||
|
||||
return redirect('/mfa/setup');
|
||||
}
|
||||
|
||||
/**
|
||||
* Show the page to start an MFA verification.
|
||||
*/
|
||||
public function verify()
|
||||
{
|
||||
$userMethods = user()->mfaValues()
|
||||
->get(['id', 'method'])
|
||||
->groupBy('method');
|
||||
|
||||
return view('mfa.verify', [
|
||||
'userMethods' => $userMethods,
|
||||
]);
|
||||
}
|
||||
}
|
||||
|
@ -48,6 +48,7 @@ class Kernel extends HttpKernel
|
||||
*/
|
||||
protected $routeMiddleware = [
|
||||
'auth' => \BookStack\Http\Middleware\Authenticate::class,
|
||||
'mfa' => \BookStack\Http\Middleware\EnforceMfaRequirements::class,
|
||||
'can' => \Illuminate\Auth\Middleware\Authorize::class,
|
||||
'guest' => \BookStack\Http\Middleware\RedirectIfAuthenticated::class,
|
||||
'throttle' => \Illuminate\Routing\Middleware\ThrottleRequests::class,
|
||||
|
@ -2,10 +2,21 @@
|
||||
|
||||
namespace BookStack\Http\Middleware;
|
||||
|
||||
use BookStack\Auth\Access\Mfa\MfaSession;
|
||||
use Closure;
|
||||
|
||||
class EnforceMfaRequirements
|
||||
{
|
||||
protected $mfaSession;
|
||||
|
||||
/**
|
||||
* EnforceMfaRequirements constructor.
|
||||
*/
|
||||
public function __construct(MfaSession $mfaSession)
|
||||
{
|
||||
$this->mfaSession = $mfaSession;
|
||||
}
|
||||
|
||||
/**
|
||||
* Handle an incoming request.
|
||||
*
|
||||
@ -15,10 +26,23 @@ class EnforceMfaRequirements
|
||||
*/
|
||||
public function handle($request, Closure $next)
|
||||
{
|
||||
$mfaRequired = user()->roles()->where('mfa_enforced', '=', true)->exists();
|
||||
// TODO - Run this after auth (If authenticated)
|
||||
// TODO - Redirect user to setup MFA or verify via MFA.
|
||||
if (
|
||||
!$this->mfaSession->isVerified()
|
||||
&& !$request->is('mfa/verify*', 'uploads/images/user/*')
|
||||
&& $this->mfaSession->requiredForCurrentUser()
|
||||
) {
|
||||
return redirect('/mfa/verify');
|
||||
}
|
||||
|
||||
// TODO - URI wildcard exceptions above allow access to the 404 page of this user
|
||||
// which could then expose content. Either need to lock that down (Tricky to do image thing)
|
||||
// or prevent any level of auth until verified.
|
||||
|
||||
// TODO - Need to redirect to setup if not configured AND ONLY IF NO OPTIONS CONFIGURED
|
||||
// Might need to change up such routes to start with /configure/ for such identification.
|
||||
// (Can't allow access to those if already configured)
|
||||
// TODO - Store mfa_pass into session for future requests?
|
||||
|
||||
return $next($request);
|
||||
}
|
||||
}
|
||||
|
31
resources/views/mfa/verify.blade.php
Normal file
31
resources/views/mfa/verify.blade.php
Normal file
@ -0,0 +1,31 @@
|
||||
@extends('simple-layout')
|
||||
|
||||
@section('body')
|
||||
<div class="container small py-xl">
|
||||
|
||||
<div class="card content-wrap auto-height">
|
||||
<h1 class="list-heading">Verify Access</h1>
|
||||
<p class="mb-none">
|
||||
Your user account requires you to confirm your identity via an additional level
|
||||
of verification before you're granted access.
|
||||
Verify using one of your configure methods to continue.
|
||||
</p>
|
||||
|
||||
<div class="setting-list">
|
||||
<div class="grid half gap-xl">
|
||||
<div>
|
||||
<div class="setting-list-label">METHOD A</div>
|
||||
<p class="small">
|
||||
...
|
||||
</p>
|
||||
</div>
|
||||
<div class="pt-m">
|
||||
<a href="{{ url('/mfa/verify/totp') }}" class="button outline">BUTTON</a>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
</div>
|
||||
|
||||
</div>
|
||||
</div>
|
||||
@stop
|
@ -4,7 +4,7 @@ Route::get('/status', 'StatusController@show');
|
||||
Route::get('/robots.txt', 'HomeController@getRobots');
|
||||
|
||||
// Authenticated routes...
|
||||
Route::group(['middleware' => 'auth'], function () {
|
||||
Route::group(['middleware' => ['auth', 'mfa']], function () {
|
||||
|
||||
// Secure images routing
|
||||
Route::get('/uploads/images/{path}', 'Images\ImageController@showImage')
|
||||
@ -224,13 +224,14 @@ Route::group(['middleware' => 'auth'], function () {
|
||||
Route::put('/roles/{id}', 'RoleController@update');
|
||||
});
|
||||
|
||||
// MFA Setup Routes
|
||||
// MFA Routes
|
||||
Route::get('/mfa/setup', 'Auth\MfaController@setup');
|
||||
Route::get('/mfa/totp-generate', 'Auth\MfaTotpController@generate');
|
||||
Route::post('/mfa/totp-confirm', 'Auth\MfaTotpController@confirm');
|
||||
Route::get('/mfa/backup-codes-generate', 'Auth\MfaBackupCodesController@generate');
|
||||
Route::post('/mfa/backup-codes-confirm', 'Auth\MfaBackupCodesController@confirm');
|
||||
Route::delete('/mfa/remove/{method}', 'Auth\MfaController@remove');
|
||||
Route::get('/mfa/verify', 'Auth\MfaController@verify');
|
||||
});
|
||||
|
||||
// Social auth routes
|
||||
|
Loading…
x
Reference in New Issue
Block a user