Added backup code setup flow

- Includes testing to cover flow.
- Moved TOTP logic to its own controller.
- Added some extra totp tests.

diff --git a/app/Auth/Access/Mfa/BackupCodeService.php b/app/Auth/Access/Mfa/BackupCodeService.php
new file mode 100644 (file)
index 0000000..cc533bd
--- /dev/null
+++ b/app/Auth/Access/Mfa/BackupCodeService.php
@@ -0,0 +1,21 @@
+<?php
+
+namespace BookStack\Auth\Access\Mfa;
+
+use Illuminate\Support\Str;
+
+class BackupCodeService
+{
+    /**
+     * Generate a new set of 16 backup codes.
+     */
+    public function generateNewSet(): array
+    {
+        $codes = [];
+        for ($i = 0; $i < 16; $i++) {
+            $code = Str::random(5) . '-' . Str::random(5);
+            $codes[] = strtolower($code);
+        }
+        return $codes;
+    }
+}
\ No newline at end of file

diff --git a/app/Auth/Access/Mfa/MfaValue.php b/app/Auth/Access/Mfa/MfaValue.php
index 6e9049c3ca10ace777511748c5d12160d61a40c4..cba90dcac2b14dcab9769bfbdbbfbd5077f129eb 100644 (file)
--- a/app/Auth/Access/Mfa/MfaValue.php
+++ b/app/Auth/Access/Mfa/MfaValue.php
@@ -19,7 +19,7 @@ class MfaValue extends Model
     protected static $unguarded = true;
 
     const METHOD_TOTP = 'totp';
     protected static $unguarded = true;
 
     const METHOD_TOTP = 'totp';

-    const METHOD_CODES = 'codes';
+    const METHOD_BACKUP_CODES = 'backup_codes';

 
     /**
      * Upsert a new MFA value for the given user and method
 
     /**
      * Upsert a new MFA value for the given user and method

diff --git a/app/Http/Controllers/Auth/MfaBackupCodesController.php b/app/Http/Controllers/Auth/MfaBackupCodesController.php
new file mode 100644 (file)
index 0000000..ba4b8f5
--- /dev/null
+++ b/app/Http/Controllers/Auth/MfaBackupCodesController.php
@@ -0,0 +1,47 @@
+<?php
+
+namespace BookStack\Http\Controllers\Auth;
+
+use BookStack\Actions\ActivityType;
+use BookStack\Auth\Access\Mfa\BackupCodeService;
+use BookStack\Auth\Access\Mfa\MfaValue;
+use BookStack\Http\Controllers\Controller;
+use Exception;
+
+class MfaBackupCodesController extends Controller
+{
+    protected const SETUP_SECRET_SESSION_KEY = 'mfa-setup-backup-codes';
+
+    /**
+     * Show a view that generates and displays backup codes
+     */
+    public function generate(BackupCodeService $codeService)
+    {
+        $codes = $codeService->generateNewSet();
+        session()->put(self::SETUP_SECRET_SESSION_KEY, encrypt($codes));
+
+        $downloadUrl = 'data:application/octet-stream;base64,' . base64_encode(implode("\n\n", $codes));
+
+        return view('mfa.backup-codes-generate', [
+            'codes' => $codes,
+            'downloadUrl' => $downloadUrl,
+        ]);
+    }
+
+    /**
+     * Confirm the setup of backup codes, storing them against the user.
+     * @throws Exception
+     */
+    public function confirm()
+    {
+        if (!session()->has(self::SETUP_SECRET_SESSION_KEY)) {
+            return response('No generated codes found in the session', 500);
+        }
+
+        $codes = decrypt(session()->pull(self::SETUP_SECRET_SESSION_KEY));
+        MfaValue::upsertWithValue(user(), MfaValue::METHOD_BACKUP_CODES, json_encode($codes));
+
+        $this->logActivity(ActivityType::MFA_SETUP_METHOD, 'backup-codes');
+        return redirect('/mfa/setup');
+    }
+}

diff --git a/app/Http/Controllers/Auth/MfaController.php b/app/Http/Controllers/Auth/MfaController.php
index 8ddccaa98cf3a18a58fd63bb52fd2d40645f7063..caee416d3c376f3124f5408a99d7c056e8ea7671 100644 (file)
--- a/app/Http/Controllers/Auth/MfaController.php
+++ b/app/Http/Controllers/Auth/MfaController.php
@@ -2,18 +2,10 @@
 
 namespace BookStack\Http\Controllers\Auth;
 
 
 namespace BookStack\Http\Controllers\Auth;
 

-use BookStack\Actions\ActivityType;
-use BookStack\Auth\Access\Mfa\MfaValue;
-use BookStack\Auth\Access\Mfa\TotpService;
-use BookStack\Auth\Access\Mfa\TotpValidationRule;

 use BookStack\Http\Controllers\Controller;
 use BookStack\Http\Controllers\Controller;

-use Illuminate\Http\Request;
-use Illuminate\Validation\ValidationException;

 
 class MfaController extends Controller
 {
 
 class MfaController extends Controller
 {

-    protected const TOTP_SETUP_SECRET_SESSION_KEY = 'mfa-setup-totp-secret';
-

     /**
      * Show the view to setup MFA for the current user.
      */
     /**
      * Show the view to setup MFA for the current user.
      */


@@ -26,47 +18,4 @@ class MfaController extends Controller
             'userMethods' => $userMethods,
         ]);
     }
             'userMethods' => $userMethods,
         ]);
     }

-
-    /**
-     * Show a view that generates and displays a TOTP QR code.
-     */
-    public function totpGenerate(TotpService $totp)
-    {
-        if (session()->has(static::TOTP_SETUP_SECRET_SESSION_KEY)) {
-            $totpSecret = decrypt(session()->get(static::TOTP_SETUP_SECRET_SESSION_KEY));
-        } else {
-            $totpSecret = $totp->generateSecret();
-            session()->put(static::TOTP_SETUP_SECRET_SESSION_KEY, encrypt($totpSecret));
-        }
-
-        $qrCodeUrl = $totp->generateUrl($totpSecret);
-        $svg = $totp->generateQrCodeSvg($qrCodeUrl);
-
-        return view('mfa.totp-generate', [
-            'secret' => $totpSecret,
-            'svg' => $svg,
-        ]);
-    }
-
-    /**
-     * Confirm the setup of TOTP and save the auth method secret
-     * against the current user.
-     * @throws ValidationException
-     */
-    public function totpConfirm(Request $request)
-    {
-        $totpSecret = decrypt(session()->get(static::TOTP_SETUP_SECRET_SESSION_KEY));
-        $this->validate($request, [
-            'code' => [
-                'required',
-                'max:12', 'min:4',
-                new TotpValidationRule($totpSecret),
-            ]
-        ]);
-
-        MfaValue::upsertWithValue(user(), MfaValue::METHOD_TOTP, $totpSecret);
-        $this->logActivity(ActivityType::MFA_SETUP_METHOD, 'totp');
-
-        return redirect('/mfa/setup');
-    }

 }
 }

diff --git a/app/Http/Controllers/Auth/MfaTotpController.php b/app/Http/Controllers/Auth/MfaTotpController.php
new file mode 100644 (file)
index 0000000..18f08e7
--- /dev/null
+++ b/app/Http/Controllers/Auth/MfaTotpController.php
@@ -0,0 +1,60 @@
+<?php
+
+namespace BookStack\Http\Controllers\Auth;
+
+use BookStack\Actions\ActivityType;
+use BookStack\Auth\Access\Mfa\MfaValue;
+use BookStack\Auth\Access\Mfa\TotpService;
+use BookStack\Auth\Access\Mfa\TotpValidationRule;
+use BookStack\Http\Controllers\Controller;
+use Illuminate\Http\Request;
+use Illuminate\Validation\ValidationException;
+
+class MfaTotpController extends Controller
+{
+    protected const SETUP_SECRET_SESSION_KEY = 'mfa-setup-totp-secret';
+
+    /**
+     * Show a view that generates and displays a TOTP QR code.
+     */
+    public function generate(TotpService $totp)
+    {
+        if (session()->has(static::SETUP_SECRET_SESSION_KEY)) {
+            $totpSecret = decrypt(session()->get(static::SETUP_SECRET_SESSION_KEY));
+        } else {
+            $totpSecret = $totp->generateSecret();
+            session()->put(static::SETUP_SECRET_SESSION_KEY, encrypt($totpSecret));
+        }
+
+        $qrCodeUrl = $totp->generateUrl($totpSecret);
+        $svg = $totp->generateQrCodeSvg($qrCodeUrl);
+
+        return view('mfa.totp-generate', [
+            'secret' => $totpSecret,
+            'svg' => $svg,
+        ]);
+    }
+
+    /**
+     * Confirm the setup of TOTP and save the auth method secret
+     * against the current user.
+     * @throws ValidationException
+     */
+    public function confirm(Request $request)
+    {
+        $totpSecret = decrypt(session()->get(static::SETUP_SECRET_SESSION_KEY));
+        $this->validate($request, [
+            'code' => [
+                'required',
+                'max:12', 'min:4',
+                new TotpValidationRule($totpSecret),
+            ]
+        ]);
+
+        MfaValue::upsertWithValue(user(), MfaValue::METHOD_TOTP, $totpSecret);
+        session()->remove(static::SETUP_SECRET_SESSION_KEY);
+        $this->logActivity(ActivityType::MFA_SETUP_METHOD, 'totp');
+
+        return redirect('/mfa/setup');
+    }
+}

diff --git a/resources/views/mfa/backup-codes-generate.blade.php b/resources/views/mfa/backup-codes-generate.blade.php
new file mode 100644 (file)
index 0000000..8b43784
--- /dev/null
+++ b/resources/views/mfa/backup-codes-generate.blade.php
@@ -0,0 +1,40 @@
+@extends('simple-layout')
+
+@section('body')
+
+    <div class="container very-small py-xl">
+        <div class="card content-wrap auto-height">
+            <h1 class="list-heading">Backup Codes</h1>
+            <p>
+                Store the below list of codes in a safe place.
+                When accessing the system you'll be able to use one of the codes
+                as a second authentication mechanism.
+            </p>
+
+            <div class="text-center mb-xs">
+                <div class="text-bigger code-base p-m" style="column-count: 2">
+                    @foreach($codes as $code)
+                        {{ $code }} <br>
+                    @endforeach
+                </div>
+            </div>
+
+            <p class="text-right">
+                <a href="{{ $downloadUrl }}" download="backup-codes.txt" class="button outline small">Download Codes</a>
+            </p>
+
+            <p class="callout warning">
+                Each code can only be used once
+            </p>
+
+            <form action="{{ url('/mfa/backup-codes-confirm') }}" method="POST">
+                {{ csrf_field() }}
+                <div class="mt-s text-right">
+                    <a href="{{ url('/mfa/setup') }}" class="button outline">{{ trans('common.cancel') }}</a>
+                    <button class="button">Confirm and Enable</button>
+                </div>
+            </form>
+        </div>
+    </div>
+
+@stop

diff --git a/resources/views/mfa/setup.blade.php b/resources/views/mfa/setup.blade.php
index d8fe50947611c536d88d38b944c0db42f1934531..c98d78885063d9c6cffb6b9b2b380484d3f3b665 100644 (file)
--- a/resources/views/mfa/setup.blade.php
+++ b/resources/views/mfa/setup.blade.php
@@ -36,12 +36,20 @@
                     <div>
                         <div class="setting-list-label">Backup Codes</div>
                         <p class="small">
                     <div>
                         <div class="setting-list-label">Backup Codes</div>
                         <p class="small">

-                            Print out or securely store a set of one-time backup codes
+                            Securely store a set of one-time-use backup codes

                             which you can enter to verify your identity.
                         </p>
                     </div>
                     <div class="pt-m">
                             which you can enter to verify your identity.
                         </p>
                     </div>
                     <div class="pt-m">

-                        <a href="{{ url('/mfa/codes/generate') }}" class="button outline">Setup</a>
+                        @if($userMethods->has('backup_codes'))
+                            <div class="text-pos">
+                                @icon('check-circle')
+                                Already configured
+                            </div>
+                            <a href="{{ url('/mfa/backup-codes-generate') }}" class="button outline small">Reconfigure</a>
+                        @else
+                            <a href="{{ url('/mfa/backup-codes-generate') }}" class="button outline">Setup</a>
+                        @endif

                     </div>
                 </div>
             </div>
                     </div>
                 </div>
             </div>

diff --git a/resources/views/mfa/totp-generate.blade.php b/resources/views/mfa/totp-generate.blade.php
index 17d38adaaa3eb6573cc189c9fadef35d4ebb15cb..c1e7547d5b9be3f49bdc6f2a113d1e70bc126cb8 100644 (file)
--- a/resources/views/mfa/totp-generate.blade.php
+++ b/resources/views/mfa/totp-generate.blade.php
@@ -35,6 +35,7 @@
                     <div class="text-neg text-small px-xs">{{ $errors->first('code') }}</div>
                 @endif
                 <div class="mt-s text-right">
                     <div class="text-neg text-small px-xs">{{ $errors->first('code') }}</div>
                 @endif
                 <div class="mt-s text-right">

+                    <a href="{{ url('/mfa/setup') }}" class="button outline">{{ trans('common.cancel') }}</a>

                     <button class="button">Confirm and Enable</button>
                 </div>
             </form>
                     <button class="button">Confirm and Enable</button>
                 </div>
             </form>

diff --git a/routes/web.php b/routes/web.php
index f9967465b3fa5cca7e7a650bd6118cae6164a460..7ab5890e0f0032c40a14a4c947bd1488d2455604 100644 (file)
--- a/routes/web.php
+++ b/routes/web.php
@@ -225,8 +225,10 @@ Route::group(['middleware' => 'auth'], function () {
     });
 
     Route::get('/mfa/setup', 'Auth\MfaController@setup');
     });
 
     Route::get('/mfa/setup', 'Auth\MfaController@setup');

-    Route::get('/mfa/totp-generate', 'Auth\MfaController@totpGenerate');
-    Route::post('/mfa/totp-confirm', 'Auth\MfaController@totpConfirm');
+    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');

 });
 
 // Social auth routes
 });
 
 // Social auth routes

diff --git a/tests/Auth/MfaConfigurationTest.php b/tests/Auth/MfaConfigurationTest.php
index 9407c3735c770029d41b44710fc74894650dafd2..870850a73bc292fbb059fbe7ab5fe6f95ebfa3ed 100644 (file)
--- a/tests/Auth/MfaConfigurationTest.php
+++ b/tests/Auth/MfaConfigurationTest.php
@@ -2,6 +2,7 @@
 
 namespace Tests\Auth;
 
 
 namespace Tests\Auth;
 

+use BookStack\Auth\Access\Mfa\MfaValue;

 use PragmaRX\Google2FA\Google2FA;
 use Tests\TestCase;
 
 use PragmaRX\Google2FA\Google2FA;
 use Tests\TestCase;
 


@@ -37,7 +38,8 @@ class MfaConfigurationTest extends TestCase
 
         // Successful confirmation
         $google2fa = new Google2FA();
 
         // Successful confirmation
         $google2fa = new Google2FA();

-        $otp = $google2fa->getCurrentOtp(decrypt(session()->get('mfa-setup-totp-secret')));
+        $secret = decrypt(session()->get('mfa-setup-totp-secret'));
+        $otp = $google2fa->getCurrentOtp($secret);

         $resp = $this->post('/mfa/totp-confirm', [
             'code' => $otp,
         ]);
         $resp = $this->post('/mfa/totp-confirm', [
             'code' => $otp,
         ]);


@@ -47,6 +49,61 @@ class MfaConfigurationTest extends TestCase
         $resp = $this->followRedirects($resp);
         $resp->assertSee('Multi-factor method successfully configured');
         $resp->assertElementContains('a[href$="/mfa/totp-generate"]', 'Reconfigure');
         $resp = $this->followRedirects($resp);
         $resp->assertSee('Multi-factor method successfully configured');
         $resp->assertElementContains('a[href$="/mfa/totp-generate"]', 'Reconfigure');

+
+        $this->assertDatabaseHas('mfa_values', [
+            'user_id' => $editor->id,
+            'method' => 'totp',
+        ]);
+        $this->assertFalse(session()->has('mfa-setup-totp-secret'));
+        $value = MfaValue::query()->where('user_id', '=', $editor->id)
+            ->where('method', '=', 'totp')->first();
+        $this->assertEquals($secret, decrypt($value->value));
+    }
+
+    public function test_backup_codes_setup()
+    {
+        $editor = $this->getEditor();
+        $this->assertDatabaseMissing('mfa_values', ['user_id' => $editor->id]);
+
+        // Setup page state
+        $resp = $this->actingAs($editor)->get('/mfa/setup');
+        $resp->assertElementContains('a[href$="/mfa/backup-codes-generate"]', 'Setup');
+
+        // Generate page access
+        $resp = $this->get('/mfa/backup-codes-generate');
+        $resp->assertSee('Backup Codes');
+        $resp->assertElementContains('form[action$="/mfa/backup-codes-confirm"]', 'Confirm and Enable');
+        $this->assertSessionHas('mfa-setup-backup-codes');
+        $codes = decrypt(session()->get('mfa-setup-backup-codes'));
+        // Check code format
+        $this->assertCount(16, $codes);
+        $this->assertEquals(16*11, strlen(implode('', $codes)));
+        // Check download link
+        $resp->assertSee(base64_encode(implode("\n\n", $codes)));
+
+        // Confirm submit
+        $resp = $this->post('/mfa/backup-codes-confirm');
+        $resp->assertRedirect('/mfa/setup');
+
+        // Confirmation of setup
+        $resp = $this->followRedirects($resp);
+        $resp->assertSee('Multi-factor method successfully configured');
+        $resp->assertElementContains('a[href$="/mfa/backup-codes-generate"]', 'Reconfigure');
+
+        $this->assertDatabaseHas('mfa_values', [
+            'user_id' => $editor->id,
+            'method' => 'backup_codes',
+        ]);
+        $this->assertFalse(session()->has('mfa-setup-backup-codes'));
+        $value = MfaValue::query()->where('user_id', '=', $editor->id)
+            ->where('method', '=', 'backup_codes')->first();
+        $this->assertEquals($codes, json_decode(decrypt($value->value)));
+    }
+
+    public function test_backup_codes_cannot_be_confirmed_if_not_previously_generated()
+    {
+        $resp = $this->asEditor()->post('/mfa/backup-codes-confirm');
+        $resp->assertStatus(500);

     }
 
 }
\ No newline at end of file
     }
 
 }
\ No newline at end of file