From 916a82616f1e2c750a1f01109e65ad2b603a79ce Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Wed, 30 Jun 2021 22:10:02 +0100
Subject: [PATCH] Complete base flow for TOTP setup

- Includes DB storage and code validation.
- Extracted TOTP work to its own service file.
- Still needs testing to cover this side of things.
---
 app/Actions/ActivityType.php                  |  2 +
 app/Auth/Access/Mfa/MfaValue.php              | 54 +++++++++++++++
 app/Auth/Access/Mfa/TotpService.php           | 66 +++++++++++++++++++
 app/Auth/Access/Mfa/TotpValidationRule.php    | 38 +++++++++++
 app/Auth/User.php                             |  9 +++
 app/Auth/UserRepo.php                         |  1 +
 app/Http/Controllers/Auth/MfaController.php   | 62 +++++++----------
 ...1_06_30_173111_create_mfa_values_table.php | 34 ++++++++++
 resources/lang/en/activities.php              |  3 +
 resources/lang/en/validation.php              |  1 +
 resources/views/mfa/setup.blade.php           | 10 ++-
 tests/Auth/MfaTotpTest.php                    | 10 +++
 12 files changed, 251 insertions(+), 39 deletions(-)
 create mode 100644 app/Auth/Access/Mfa/MfaValue.php
 create mode 100644 app/Auth/Access/Mfa/TotpService.php
 create mode 100644 app/Auth/Access/Mfa/TotpValidationRule.php
 create mode 100644 database/migrations/2021_06_30_173111_create_mfa_values_table.php
 create mode 100644 tests/Auth/MfaTotpTest.php

diff --git a/app/Actions/ActivityType.php b/app/Actions/ActivityType.php
index a19f143b7..e2e7b5a5d 100644
--- a/app/Actions/ActivityType.php
+++ b/app/Actions/ActivityType.php
@@ -50,4 +50,6 @@ class ActivityType
     const AUTH_PASSWORD_RESET_UPDATE = 'auth_password_reset_update';
     const AUTH_LOGIN = 'auth_login';
     const AUTH_REGISTER = 'auth_register';
+
+    const MFA_SETUP_METHOD = 'mfa_setup_method';
 }
diff --git a/app/Auth/Access/Mfa/MfaValue.php b/app/Auth/Access/Mfa/MfaValue.php
new file mode 100644
index 000000000..6e9049c3c
--- /dev/null
+++ b/app/Auth/Access/Mfa/MfaValue.php
@@ -0,0 +1,54 @@
+<?php
+
+namespace BookStack\Auth\Access\Mfa;
+
+use BookStack\Auth\User;
+use Carbon\Carbon;
+use Illuminate\Database\Eloquent\Model;
+
+/**
+ * @property int $id
+ * @property int $user_id
+ * @property string $method
+ * @property string $value
+ * @property Carbon $created_at
+ * @property Carbon $updated_at
+ */
+class MfaValue extends Model
+{
+    protected static $unguarded = true;
+
+    const METHOD_TOTP = 'totp';
+    const METHOD_CODES = 'codes';
+
+    /**
+     * Upsert a new MFA value for the given user and method
+     * using the provided value.
+     */
+    public static function upsertWithValue(User $user, string $method, string $value): void
+    {
+        /** @var MfaValue $mfaVal */
+        $mfaVal = static::query()->firstOrNew([
+            'user_id' => $user->id,
+            'method' => $method
+        ]);
+        $mfaVal->setValue($value);
+        $mfaVal->save();
+    }
+
+    /**
+     * Decrypt the value attribute upon access.
+     */
+    public function getValue(): string
+    {
+        return decrypt($this->value);
+    }
+
+    /**
+     * Encrypt the value attribute upon access.
+     */
+    public function setValue($value): void
+    {
+        $this->value = encrypt($value);
+    }
+}
diff --git a/app/Auth/Access/Mfa/TotpService.php b/app/Auth/Access/Mfa/TotpService.php
new file mode 100644
index 000000000..f9a9f416e
--- /dev/null
+++ b/app/Auth/Access/Mfa/TotpService.php
@@ -0,0 +1,66 @@
+<?php
+
+namespace BookStack\Auth\Access\Mfa;
+
+use BaconQrCode\Renderer\Color\Rgb;
+use BaconQrCode\Renderer\Image\SvgImageBackEnd;
+use BaconQrCode\Renderer\ImageRenderer;
+use BaconQrCode\Renderer\RendererStyle\Fill;
+use BaconQrCode\Renderer\RendererStyle\RendererStyle;
+use BaconQrCode\Writer;
+use PragmaRX\Google2FA\Google2FA;
+
+class TotpService
+{
+    protected $google2fa;
+
+    public function __construct(Google2FA $google2fa)
+    {
+        $this->google2fa = $google2fa;
+    }
+
+    /**
+     * Generate a new totp secret key.
+     */
+    public function generateSecret(): string
+    {
+        /** @noinspection PhpUnhandledExceptionInspection */
+        return $this->google2fa->generateSecretKey();
+    }
+
+    /**
+     * Generate a TOTP URL from secret key.
+     */
+    public function generateUrl(string $secret): string
+    {
+        return $this->google2fa->getQRCodeUrl(
+            setting('app-name'),
+            user()->email,
+            $secret
+        );
+    }
+
+    /**
+     * Generate a QR code to display a TOTP URL.
+     */
+    public function generateQrCodeSvg(string $url): string
+    {
+        $color = Fill::uniformColor(new Rgb(255, 255, 255), new Rgb(32, 110, 167));
+        return (new Writer(
+            new ImageRenderer(
+                new RendererStyle(192, 0, null, null, $color),
+                new SvgImageBackEnd
+            )
+        ))->writeString($url);
+    }
+
+    /**
+     * Verify that the user provided code is valid for the secret.
+     * The secret must be known, not user-provided.
+     */
+    public function verifyCode(string $code, string $secret): bool
+    {
+        /** @noinspection PhpUnhandledExceptionInspection */
+        return $this->google2fa->verifyKey($secret, $code);
+    }
+}
\ No newline at end of file
diff --git a/app/Auth/Access/Mfa/TotpValidationRule.php b/app/Auth/Access/Mfa/TotpValidationRule.php
new file mode 100644
index 000000000..7fe3d8504
--- /dev/null
+++ b/app/Auth/Access/Mfa/TotpValidationRule.php
@@ -0,0 +1,38 @@
+<?php
+
+namespace BookStack\Auth\Access\Mfa;
+
+use Illuminate\Contracts\Validation\Rule;
+
+class TotpValidationRule implements Rule
+{
+
+    protected $secret;
+    protected $totpService;
+
+    /**
+     * Create a new rule instance.
+     * Takes the TOTP secret that must be system provided, not user provided.
+     */
+    public function __construct(string $secret)
+    {
+        $this->secret = $secret;
+        $this->totpService = app()->make(TotpService::class);
+    }
+
+    /**
+     * Determine if the validation rule passes.
+     */
+    public function passes($attribute, $value)
+    {
+        return $this->totpService->verifyCode($value, $this->secret);
+    }
+
+    /**
+     * Get the validation error message.
+     */
+    public function message()
+    {
+        return trans('validation.totp');
+    }
+}
diff --git a/app/Auth/User.php b/app/Auth/User.php
index 1a3560691..f4fd45281 100644
--- a/app/Auth/User.php
+++ b/app/Auth/User.php
@@ -4,6 +4,7 @@ namespace BookStack\Auth;
 
 use BookStack\Actions\Favourite;
 use BookStack\Api\ApiToken;
+use BookStack\Auth\Access\Mfa\MfaValue;
 use BookStack\Entities\Tools\SlugGenerator;
 use BookStack\Interfaces\Loggable;
 use BookStack\Interfaces\Sluggable;
@@ -265,6 +266,14 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon
         return $this->hasMany(Favourite::class);
     }
 
+    /**
+     * Get the MFA values belonging to this use.
+     */
+    public function mfaValues(): HasMany
+    {
+        return $this->hasMany(MfaValue::class);
+    }
+
     /**
      * Get the last activity time for this user.
      */
diff --git a/app/Auth/UserRepo.php b/app/Auth/UserRepo.php
index 61ca12dcc..9faeb8ae2 100644
--- a/app/Auth/UserRepo.php
+++ b/app/Auth/UserRepo.php
@@ -188,6 +188,7 @@ class UserRepo
         $user->socialAccounts()->delete();
         $user->apiTokens()->delete();
         $user->favourites()->delete();
+        $user->mfaValues()->delete();
         $user->delete();
 
         // Delete user profile images
diff --git a/app/Http/Controllers/Auth/MfaController.php b/app/Http/Controllers/Auth/MfaController.php
index be381e3b0..8ddccaa98 100644
--- a/app/Http/Controllers/Auth/MfaController.php
+++ b/app/Http/Controllers/Auth/MfaController.php
@@ -2,19 +2,13 @@
 
 namespace BookStack\Http\Controllers\Auth;
 
-use BaconQrCode\Renderer\Color\Rgb;
-use BaconQrCode\Renderer\Image\SvgImageBackEnd;
-use BaconQrCode\Renderer\ImageRenderer;
-use BaconQrCode\Renderer\RendererStyle\Fill;
-use BaconQrCode\Renderer\RendererStyle\RendererStyle;
-use BaconQrCode\Writer;
+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;
-use PragmaRX\Google2FA\Exceptions\IncompatibleWithGoogleAuthenticatorException;
-use PragmaRX\Google2FA\Exceptions\InvalidCharactersException;
-use PragmaRX\Google2FA\Exceptions\SecretKeyTooShortException;
-use PragmaRX\Google2FA\Google2FA;
 
 class MfaController extends Controller
 {
@@ -25,44 +19,29 @@ class MfaController extends Controller
      */
     public function setup()
     {
-        // TODO - Redirect back to profile/edit if already setup?
-        // Show MFA setup route
-        return view('mfa.setup');
+        $userMethods = user()->mfaValues()
+            ->get(['id', 'method'])
+            ->groupBy('method');
+        return view('mfa.setup', [
+            'userMethods' => $userMethods,
+        ]);
     }
 
     /**
      * Show a view that generates and displays a TOTP QR code.
-     * @throws IncompatibleWithGoogleAuthenticatorException
-     * @throws InvalidCharactersException
-     * @throws SecretKeyTooShortException
      */
-    public function totpGenerate()
+    public function totpGenerate(TotpService $totp)
     {
-        // TODO - Ensure a QR code doesn't already exist? Or overwrite?
-        $google2fa = new Google2FA();
         if (session()->has(static::TOTP_SETUP_SECRET_SESSION_KEY)) {
             $totpSecret = decrypt(session()->get(static::TOTP_SETUP_SECRET_SESSION_KEY));
         } else {
-            $totpSecret = $google2fa->generateSecretKey();
+            $totpSecret = $totp->generateSecret();
             session()->put(static::TOTP_SETUP_SECRET_SESSION_KEY, encrypt($totpSecret));
         }
 
-        $qrCodeUrl = $google2fa->getQRCodeUrl(
-            setting('app-name'),
-            user()->email,
-            $totpSecret
-        );
+        $qrCodeUrl = $totp->generateUrl($totpSecret);
+        $svg = $totp->generateQrCodeSvg($qrCodeUrl);
 
-        $color = Fill::uniformColor(new Rgb(255, 255, 255), new Rgb(32, 110, 167));
-        $svg = (new Writer(
-            new ImageRenderer(
-                new RendererStyle(192, 0, null, null, $color),
-                new SvgImageBackEnd
-            )
-        ))->writeString($qrCodeUrl);
-
-        // Get user to verify setup via responding once.
-        // If correct response, Save key against user
         return view('mfa.totp-generate', [
             'secret' => $totpSecret,
             'svg' => $svg,
@@ -76,11 +55,18 @@ class MfaController extends Controller
      */
     public function totpConfirm(Request $request)
     {
+        $totpSecret = decrypt(session()->get(static::TOTP_SETUP_SECRET_SESSION_KEY));
         $this->validate($request, [
-            'code' => 'required|max:12|min:4'
+            'code' => [
+                'required',
+                'max:12', 'min:4',
+                new TotpValidationRule($totpSecret),
+            ]
         ]);
 
-        // TODO - Confirm code
-        dd($request->input('code'));
+        MfaValue::upsertWithValue(user(), MfaValue::METHOD_TOTP, $totpSecret);
+        $this->logActivity(ActivityType::MFA_SETUP_METHOD, 'totp');
+
+        return redirect('/mfa/setup');
     }
 }
diff --git a/database/migrations/2021_06_30_173111_create_mfa_values_table.php b/database/migrations/2021_06_30_173111_create_mfa_values_table.php
new file mode 100644
index 000000000..937fd31d9
--- /dev/null
+++ b/database/migrations/2021_06_30_173111_create_mfa_values_table.php
@@ -0,0 +1,34 @@
+<?php
+
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Support\Facades\Schema;
+
+class CreateMfaValuesTable extends Migration
+{
+    /**
+     * Run the migrations.
+     *
+     * @return void
+     */
+    public function up()
+    {
+        Schema::create('mfa_values', function (Blueprint $table) {
+            $table->increments('id');
+            $table->integer('user_id')->index();
+            $table->string('method', 20)->index();
+            $table->text('value');
+            $table->timestamps();
+        });
+    }
+
+    /**
+     * Reverse the migrations.
+     *
+     * @return void
+     */
+    public function down()
+    {
+        Schema::dropIfExists('mfa_values');
+    }
+}
diff --git a/resources/lang/en/activities.php b/resources/lang/en/activities.php
index 5917de2cf..2c371729b 100644
--- a/resources/lang/en/activities.php
+++ b/resources/lang/en/activities.php
@@ -47,6 +47,9 @@ return [
     'favourite_add_notification' => '":name" has been added to your favourites',
     'favourite_remove_notification' => '":name" has been removed from your favourites',
 
+    // MFA
+    'mfa_setup_method_notification' => 'Multi-factor method successfully configured',
+
     // Other
     'commented_on'                => 'commented on',
     'permissions_update'          => 'updated permissions',
diff --git a/resources/lang/en/validation.php b/resources/lang/en/validation.php
index 4031de2ae..5796d04c6 100644
--- a/resources/lang/en/validation.php
+++ b/resources/lang/en/validation.php
@@ -98,6 +98,7 @@ return [
     ],
     'string'               => 'The :attribute must be a string.',
     'timezone'             => 'The :attribute must be a valid zone.',
+    'totp'                 => 'The provided code is not valid or has expired.',
     'unique'               => 'The :attribute has already been taken.',
     'url'                  => 'The :attribute format is invalid.',
     'uploaded'             => 'The file could not be uploaded. The server may not accept files of this size.',
diff --git a/resources/views/mfa/setup.blade.php b/resources/views/mfa/setup.blade.php
index 577841af5..d8fe50947 100644
--- a/resources/views/mfa/setup.blade.php
+++ b/resources/views/mfa/setup.blade.php
@@ -20,7 +20,15 @@
                         </p>
                     </div>
                     <div class="pt-m">
-                        <a href="{{ url('/mfa/totp-generate') }}" class="button outline">Setup</a>
+                        @if($userMethods->has('totp'))
+                            <div class="text-pos">
+                                @icon('check-circle')
+                                Already configured
+                            </div>
+                            <a href="{{ url('/mfa/totp-generate') }}" class="button outline small">Reconfigure</a>
+                        @else
+                            <a href="{{ url('/mfa/totp-generate') }}" class="button outline">Setup</a>
+                        @endif
                     </div>
                 </div>
 
diff --git a/tests/Auth/MfaTotpTest.php b/tests/Auth/MfaTotpTest.php
new file mode 100644
index 000000000..f922063d6
--- /dev/null
+++ b/tests/Auth/MfaTotpTest.php
@@ -0,0 +1,10 @@
+<?php
+
+namespace Tests\Auth;
+
+use Tests\TestCase;
+
+class MfaTotpTest extends TestCase
+{
+    // TODO
+}
\ No newline at end of file