diff --git a/app/Actions/ActivityType.php b/app/Actions/ActivityType.php index e2e7b5a5d..60b1630e0 100644 --- a/app/Actions/ActivityType.php +++ b/app/Actions/ActivityType.php @@ -52,4 +52,5 @@ class ActivityType const AUTH_REGISTER = 'auth_register'; const MFA_SETUP_METHOD = 'mfa_setup_method'; + const MFA_REMOVE_METHOD = 'mfa_remove_method'; } diff --git a/app/Auth/Access/Mfa/MfaValue.php b/app/Auth/Access/Mfa/MfaValue.php index cba90dcac..9f9ab29a5 100644 --- a/app/Auth/Access/Mfa/MfaValue.php +++ b/app/Auth/Access/Mfa/MfaValue.php @@ -21,6 +21,14 @@ class MfaValue extends Model const METHOD_TOTP = 'totp'; const METHOD_BACKUP_CODES = 'backup_codes'; + /** + * Get all the MFA methods available. + */ + public static function allMethods(): array + { + return [self::METHOD_TOTP, self::METHOD_BACKUP_CODES]; + } + /** * Upsert a new MFA value for the given user and method * using the provided value. diff --git a/app/Http/Controllers/Auth/MfaController.php b/app/Http/Controllers/Auth/MfaController.php index caee416d3..9feda9433 100644 --- a/app/Http/Controllers/Auth/MfaController.php +++ b/app/Http/Controllers/Auth/MfaController.php @@ -2,6 +2,8 @@ namespace BookStack\Http\Controllers\Auth; +use BookStack\Actions\ActivityType; +use BookStack\Auth\Access\Mfa\MfaValue; use BookStack\Http\Controllers\Controller; class MfaController extends Controller @@ -18,4 +20,21 @@ class MfaController extends Controller 'userMethods' => $userMethods, ]); } + + /** + * Remove an MFA method for the current user. + * @throws \Exception + */ + public function remove(string $method) + { + if (in_array($method, MfaValue::allMethods())) { + $value = user()->mfaValues()->where('method', '=', $method)->first(); + if ($value) { + $value->delete(); + $this->logActivity(ActivityType::MFA_REMOVE_METHOD, $method); + } + } + + return redirect('/mfa/setup'); + } } diff --git a/resources/lang/en/activities.php b/resources/lang/en/activities.php index 2c371729b..50bda60bd 100644 --- a/resources/lang/en/activities.php +++ b/resources/lang/en/activities.php @@ -49,6 +49,7 @@ return [ // MFA 'mfa_setup_method_notification' => 'Multi-factor method successfully configured', + 'mfa_remove_method_notification' => 'Multi-factor method successfully removed', // Other 'commented_on' => 'commented on', diff --git a/resources/sass/_layout.scss b/resources/sass/_layout.scss index 516d7d612..e26948301 100644 --- a/resources/sass/_layout.scss +++ b/resources/sass/_layout.scss @@ -181,6 +181,10 @@ body.flexbox { display: inline-block !important; } +.relative { + position: relative; +} + .hidden { display: none !important; } diff --git a/resources/views/mfa/setup.blade.php b/resources/views/mfa/setup.blade.php index c98d78885..2ec8d0f77 100644 --- a/resources/views/mfa/setup.blade.php +++ b/resources/views/mfa/setup.blade.php @@ -26,6 +26,17 @@ Already configured </div> <a href="{{ url('/mfa/totp-generate') }}" class="button outline small">Reconfigure</a> + <div component="dropdown" class="inline relative"> + <button type="button" refs="dropdown@toggle" class="button outline small">Remove</button> + <div refs="dropdown@menu" class="dropdown-menu"> + <p class="text-neg small px-m mb-xs">Are you sure you want to remove this multi-factor authentication method?</p> + <form action="{{ url('/mfa/remove/totp') }}" method="post"> + {{ csrf_field() }} + {{ method_field('delete') }} + <button class="text-primary small delete">{{ trans('common.confirm') }}</button> + </form> + </div> + </div> @else <a href="{{ url('/mfa/totp-generate') }}" class="button outline">Setup</a> @endif @@ -47,6 +58,17 @@ Already configured </div> <a href="{{ url('/mfa/backup-codes-generate') }}" class="button outline small">Reconfigure</a> + <div component="dropdown" class="inline relative"> + <button type="button" refs="dropdown@toggle" class="button outline small">Remove</button> + <div refs="dropdown@menu" class="dropdown-menu"> + <p class="text-neg small px-m mb-xs">Are you sure you want to remove this multi-factor authentication method?</p> + <form action="{{ url('/mfa/remove/backup_codes') }}" method="post"> + {{ csrf_field() }} + {{ method_field('delete') }} + <button class="text-primary small delete">{{ trans('common.confirm') }}</button> + </form> + </div> + </div> @else <a href="{{ url('/mfa/backup-codes-generate') }}" class="button outline">Setup</a> @endif diff --git a/routes/web.php b/routes/web.php index 3be6218b0..e3329edc4 100644 --- a/routes/web.php +++ b/routes/web.php @@ -230,6 +230,7 @@ Route::group(['middleware' => 'auth'], function () { 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'); }); // Social auth routes diff --git a/tests/Auth/MfaConfigurationTest.php b/tests/Auth/MfaConfigurationTest.php index adeb66189..a8ca815fb 100644 --- a/tests/Auth/MfaConfigurationTest.php +++ b/tests/Auth/MfaConfigurationTest.php @@ -2,6 +2,7 @@ namespace Tests\Auth; +use BookStack\Actions\ActivityType; use BookStack\Auth\Access\Mfa\MfaValue; use BookStack\Auth\User; use PragmaRX\Google2FA\Google2FA; @@ -145,4 +146,22 @@ class MfaConfigurationTest extends TestCase $resp->assertElementExists('[title="MFA Configured"] svg'); } + public function test_remove_mfa_method() + { + $admin = $this->getAdmin(); + + MfaValue::upsertWithValue($admin, MfaValue::METHOD_TOTP, 'test'); + $this->assertEquals(1, $admin->mfaValues()->count()); + $resp = $this->actingAs($admin)->get('/mfa/setup'); + $resp->assertElementExists('form[action$="/mfa/remove/totp"]'); + + $resp = $this->delete("/mfa/remove/totp"); + $resp->assertRedirect("/mfa/setup"); + $resp = $this->followRedirects($resp); + $resp->assertSee('Multi-factor method successfully removed'); + + $this->assertActivityExists(ActivityType::MFA_REMOVE_METHOD); + $this->assertEquals(0, $admin->mfaValues()->count()); + } + } \ No newline at end of file