From cfc0c593dba2c0b4ce31b98e13d90779f8c50453 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Wed, 14 Jul 2021 20:18:48 +0100 Subject: [PATCH] Added MFA indicator to user list Also fixed issue with showing incorrect MFA method count on user edit page changes done in last commit --- app/Auth/User.php | 1 + app/Auth/UserRepo.php | 1 + app/Http/Controllers/UserController.php | 5 +++-- resources/views/users/index.blade.php | 7 ++++++- tests/Auth/MfaConfigurationTest.php | 26 +++++++++++++++++++------ 5 files changed, 31 insertions(+), 9 deletions(-) diff --git a/app/Auth/User.php b/app/Auth/User.php index f4fd45281..0a6849fe0 100644 --- a/app/Auth/User.php +++ b/app/Auth/User.php @@ -39,6 +39,7 @@ use Illuminate\Support\Collection; * @property string $external_auth_id * @property string $system_name * @property Collection $roles + * @property Collection $mfaValues */ class User extends Model implements AuthenticatableContract, CanResetPasswordContract, Loggable, Sluggable { diff --git a/app/Auth/UserRepo.php b/app/Auth/UserRepo.php index 9faeb8ae2..e1a040fc2 100644 --- a/app/Auth/UserRepo.php +++ b/app/Auth/UserRepo.php @@ -71,6 +71,7 @@ class UserRepo $query = User::query()->select(['*']) ->withLastActivityAt() ->with(['roles', 'avatar']) + ->withCount('mfaValues') ->orderBy($sort, $sortData['order']); if ($sortData['search']) { diff --git a/app/Http/Controllers/UserController.php b/app/Http/Controllers/UserController.php index 7f42e94cc..a0da220ee 100644 --- a/app/Http/Controllers/UserController.php +++ b/app/Http/Controllers/UserController.php @@ -123,12 +123,13 @@ class UserController extends Controller { $this->checkPermissionOrCurrentUser('users-manage', $id); - $user = $this->user->newQuery()->with(['apiTokens'])->findOrFail($id); + /** @var User $user */ + $user = $this->user->newQuery()->with(['apiTokens', 'mfaValues'])->findOrFail($id); $authMethod = ($user->system_name) ? 'system' : config('auth.method'); $activeSocialDrivers = $socialAuthService->getActiveDrivers(); - $mfaMethods = user()->mfaValues()->get(['id', 'method'])->groupBy('method'); + $mfaMethods = $user->mfaValues->groupBy('method'); $this->setPageTitle(trans('settings.user_profile')); $roles = $this->userRepo->getAllRoles(); diff --git a/resources/views/users/index.blade.php b/resources/views/users/index.blade.php index 5eef51175..9a9221242 100644 --- a/resources/views/users/index.blade.php +++ b/resources/views/users/index.blade.php @@ -43,7 +43,12 @@ {{ $user->name }} id}") }}"> - {{ $user->name }}
{{ $user->email }} + {{ $user->name }} +
+ {{ $user->email }} + @if($user->mfa_values_count > 0) + @icon('lock') + @endif
diff --git a/tests/Auth/MfaConfigurationTest.php b/tests/Auth/MfaConfigurationTest.php index f332b6721..adeb66189 100644 --- a/tests/Auth/MfaConfigurationTest.php +++ b/tests/Auth/MfaConfigurationTest.php @@ -3,6 +3,7 @@ namespace Tests\Auth; use BookStack\Auth\Access\Mfa\MfaValue; +use BookStack\Auth\User; use PragmaRX\Google2FA\Google2FA; use Tests\TestCase; @@ -108,16 +109,16 @@ class MfaConfigurationTest extends TestCase public function test_mfa_method_count_is_visible_on_user_edit_page() { - $admin = $this->getAdmin(); - $resp = $this->actingAs($admin)->get($admin->getEditUrl()); + $user = $this->getEditor(); + $resp = $this->actingAs($this->getAdmin())->get($user->getEditUrl()); $resp->assertSee('0 methods configured'); - MfaValue::upsertWithValue($admin, MfaValue::METHOD_TOTP, 'test'); - $resp = $this->actingAs($admin)->get($admin->getEditUrl()); + MfaValue::upsertWithValue($user, MfaValue::METHOD_TOTP, 'test'); + $resp = $this->get($user->getEditUrl()); $resp->assertSee('1 method configured'); - MfaValue::upsertWithValue($admin, MfaValue::METHOD_BACKUP_CODES, 'test'); - $resp = $this->actingAs($admin)->get($admin->getEditUrl()); + MfaValue::upsertWithValue($user, MfaValue::METHOD_BACKUP_CODES, 'test'); + $resp = $this->get($user->getEditUrl()); $resp->assertSee('2 methods configured'); } @@ -131,4 +132,17 @@ class MfaConfigurationTest extends TestCase $resp->assertElementNotExists('a[href$="/mfa/setup"]'); } + public function test_mfa_indicator_shows_in_user_list() + { + $admin = $this->getAdmin(); + User::query()->where('id', '!=', $admin->id)->delete(); + + $resp = $this->actingAs($admin)->get('/settings/users'); + $resp->assertElementNotExists('[title="MFA Configured"] svg'); + + MfaValue::upsertWithValue($admin, MfaValue::METHOD_TOTP, 'test'); + $resp = $this->actingAs($admin)->get('/settings/users'); + $resp->assertElementExists('[title="MFA Configured"] svg'); + } + } \ No newline at end of file