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.

diff --git a/app/Actions/ActivityType.php b/app/Actions/ActivityType.php
index a19f143b7be0d3dacfc5f46b865db67f9c83c084..e2e7b5a5d29ed2757f1487b2e7e8e594222a63cd 100644 (file)
--- 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 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 (file)
index 0000000..6e9049c
--- /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 (file)
index 0000000..f9a9f41
--- /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 (file)
index 0000000..7fe3d85
--- /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 1a3560691d4db8792c2fe18f52322c608f956dac..f4fd4528114dd89ff9d485aeb518ea7865f48e87 100644 (file)
--- 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\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;
 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);
     }
 
         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.
      */
     /**
      * Get the last activity time for this user.
      */

diff --git a/app/Auth/UserRepo.php b/app/Auth/UserRepo.php
index 61ca12dcc8f7d00e410170e0567e3b107ca0dc86..9faeb8ae27ea4ff4671381df0db1a0e0b7e9f56e 100644 (file)
--- 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->socialAccounts()->delete();
         $user->apiTokens()->delete();
         $user->favourites()->delete();

+        $user->mfaValues()->delete();

         $user->delete();
 
         // Delete user profile images
         $user->delete();
 
         // Delete user profile images

diff --git a/app/Http/Controllers/Auth/MfaController.php b/app/Http/Controllers/Auth/MfaController.php
index be381e3b0d5e23271e6b030ab911e3ce97e2fbb6..8ddccaa98cf3a18a58fd63bb52fd2d40645f7063 100644 (file)
--- a/app/Http/Controllers/Auth/MfaController.php
+++ b/app/Http/Controllers/Auth/MfaController.php
@@ -2,19 +2,13 @@
 
 namespace BookStack\Http\Controllers\Auth;
 
 
 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 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
 {
 
 class MfaController extends Controller
 {


@@ -25,44 +19,29 @@ class MfaController extends Controller
      */
     public function setup()
     {
      */
     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.
     }
 
     /**
      * 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 {
         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));
         }
 
             session()->put(static::TOTP_SETUP_SECRET_SESSION_KEY, encrypt($totpSecret));
         }
 

-        $qrCodeUrl = $google2fa->getQRCodeUrl(
-            setting('app-name'),
-            user()->email,
-            $totpSecret
-        );
-
-        $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);
+        $qrCodeUrl = $totp->generateUrl($totpSecret);
+        $svg = $totp->generateQrCodeSvg($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,
         return view('mfa.totp-generate', [
             'secret' => $totpSecret,
             'svg' => $svg,


@@ -76,11 +55,18 @@ class MfaController extends Controller
      */
     public function totpConfirm(Request $request)
     {
      */
     public function totpConfirm(Request $request)
     {

+        $totpSecret = decrypt(session()->get(static::TOTP_SETUP_SECRET_SESSION_KEY));

         $this->validate($request, [
         $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 (file)
index 0000000..937fd31
--- /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 5917de2cfb25745e4cda5a8046f310ec19da885b..2c371729ba6ea3659948a5103f9420eeb718cbe6 100644 (file)
--- 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',
 
     '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',
     // Other
     'commented_on'                => 'commented on',
     'permissions_update'          => 'updated permissions',

diff --git a/resources/lang/en/validation.php b/resources/lang/en/validation.php
index 4031de2ae743b75bafd49195ff06b29a347cbf22..5796d04c6a76c69aeb5f29e866beef5b3ee450fa 100644 (file)
--- 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.',
     ],
     '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.',
     '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 577841af5d2e7210f2981e50f123017f51dd9844..d8fe50947611c536d88d38b944c0db42f1934531 100644 (file)
--- a/resources/views/mfa/setup.blade.php
+++ b/resources/views/mfa/setup.blade.php
@@ -20,7 +20,15 @@
                         </p>
                     </div>
                     <div class="pt-m">
                         </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>
 
                     </div>
                 </div>
 

diff --git a/tests/Auth/MfaTotpTest.php b/tests/Auth/MfaTotpTest.php
new file mode 100644 (file)
index 0000000..f922063
--- /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