Added the ability to remove an MFA method

Includes testing to cover

diff --git a/app/Actions/ActivityType.php b/app/Actions/ActivityType.php
index e2e7b5a5d29ed2757f1487b2e7e8e594222a63cd..60b1630e075121693e4084f7940bd5ac9b793fb1 100644 (file)
--- 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 cba90dcac2b14dcab9769bfbdbbfbd5077f129eb..9f9ab29a5012d0fa2eb5b4d2a1c8cf84f673ac9b 100644 (file)
--- 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 caee416d3c376f3124f5408a99d7c056e8ea7671..9feda9433aefda07d09d6203c31371da88f06b7a 100644 (file)
--- 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 2c371729ba6ea3659948a5103f9420eeb718cbe6..50bda60bd294e2070a365b30047a0774aade83ae 100644 (file)
--- 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 516d7d612e590d7a6ec516a689cdd102ff6606c7..e26948301f79b5d0e9c5c120c3448d50bb413ae7 100644 (file)
--- 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 c98d78885063d9c6cffb6b9b2b380484d3f3b665..2ec8d0f775254f3a4ad4fff45bbe272eb8e9ff08 100644 (file)
--- 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 3be6218b0518b1ff1d5b896fb71821b7851a0c22..e3329edc48ca2d3b37eb03f03f14e141f2053daa 100644 (file)
--- 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 adeb66189011ec59f4db38f9de081e100dad88ea..a8ca815fbeb0a52ebcf20ba7dd0f59e07c49713e 100644 (file)
--- 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