From 17215431caec61334cd653e030ba3b42c7ef2ca1 Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Mon, 31 Jan 2022 14:16:56 +0000
Subject: [PATCH] Fixed default registration role display options

- This also allows an admin to choose not to have a default role.
- Also applied latest styleCI fixes.

For #3220
---
 app/Auth/User.php                        |  2 +-
 app/Config/dompdf.php                    |  3 +--
 app/Config/snappy.php                    |  5 ++---
 resources/lang/en/common.php             |  1 +
 resources/views/settings/index.blade.php |  3 ++-
 tests/Auth/AuthTest.php                  | 19 ++++++++++++++++++-
 6 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/app/Auth/User.php b/app/Auth/User.php
index 540a8d7ab..f969b351f 100644
--- a/app/Auth/User.php
+++ b/app/Auth/User.php
@@ -146,7 +146,7 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon
      */
     public function attachDefaultRole(): void
     {
-        $roleId = setting('registration-role');
+        $roleId = intval(setting('registration-role'));
         if ($roleId && $this->roles()->where('id', '=', $roleId)->count() === 0) {
             $this->roles()->attach($roleId);
         }
diff --git a/app/Config/dompdf.php b/app/Config/dompdf.php
index 55260a22a..a8728852c 100644
--- a/app/Config/dompdf.php
+++ b/app/Config/dompdf.php
@@ -7,9 +7,8 @@
  * Configuration should be altered via the `.env` file or environment variables.
  * Do not edit this file unless you're happy to maintain any changes yourself.
  */
-
 $dompdfPaperSizeMap = [
-    'a4' => 'a4',
+    'a4'     => 'a4',
     'letter' => 'letter',
 ];
 
diff --git a/app/Config/snappy.php b/app/Config/snappy.php
index abdec8832..8ab10a39c 100644
--- a/app/Config/snappy.php
+++ b/app/Config/snappy.php
@@ -7,9 +7,8 @@
  * Configuration should be altered via the `.env` file or environment variables.
  * Do not edit this file unless you're happy to maintain any changes yourself.
  */
-
 $snappyPaperSizeMap = [
-    'a4' => 'A4',
+    'a4'     => 'A4',
     'letter' => 'Letter',
 ];
 
@@ -19,7 +18,7 @@ return [
         'binary'  => file_exists(base_path('wkhtmltopdf')) ? base_path('wkhtmltopdf') : env('WKHTMLTOPDF', false),
         'timeout' => false,
         'options' => [
-            'outline' => true,
+            'outline'   => true,
             'page-size' => $snappyPaperSizeMap[env('EXPORT_PAGE_SIZE', 'a4')] ?? 'A4',
         ],
         'env'     => [],
diff --git a/resources/lang/en/common.php b/resources/lang/en/common.php
index 013042134..2f09e53d1 100644
--- a/resources/lang/en/common.php
+++ b/resources/lang/en/common.php
@@ -75,6 +75,7 @@ return [
     'status_active' => 'Active',
     'status_inactive' => 'Inactive',
     'never' => 'Never',
+    'none' => 'None',
 
     // Header
     'header_menu_expand' => 'Expand Header Menu',
diff --git a/resources/views/settings/index.blade.php b/resources/views/settings/index.blade.php
index 5fe5f3685..8b5615658 100644
--- a/resources/views/settings/index.blade.php
+++ b/resources/views/settings/index.blade.php
@@ -227,10 +227,11 @@
 
                             <label for="setting-registration-role">{{ trans('settings.reg_default_role') }}</label>
                             <select id="setting-registration-role" name="setting-registration-role" @if($errors->has('setting-registration-role')) class="neg" @endif>
+                                <option value="0" @if(intval(setting('registration-role', '0')) === 0) selected @endif>-- {{ trans('common.none') }} --</option>
                                 @foreach(\BookStack\Auth\Role::all() as $role)
                                     <option value="{{$role->id}}"
                                             data-system-role-name="{{ $role->system_name ?? '' }}"
-                                            @if(setting('registration-role', \BookStack\Auth\Role::first()->id) == $role->id) selected @endif
+                                            @if(intval(setting('registration-role', '0')) === $role->id) selected @endif
                                     >
                                         {{ $role->display_name }}
                                     </option>
diff --git a/tests/Auth/AuthTest.php b/tests/Auth/AuthTest.php
index 2c48131a8..fd953021d 100644
--- a/tests/Auth/AuthTest.php
+++ b/tests/Auth/AuthTest.php
@@ -3,6 +3,7 @@
 namespace Tests\Auth;
 
 use BookStack\Auth\Access\Mfa\MfaSession;
+use BookStack\Auth\Role;
 use BookStack\Auth\User;
 use BookStack\Entities\Models\Page;
 use BookStack\Notifications\ConfirmEmail;
@@ -43,7 +44,10 @@ class AuthTest extends TestCase
     public function test_normal_registration()
     {
         // Set settings and get user instance
-        $this->setSettings(['registration-enabled' => 'true']);
+        /** @var Role $registrationRole */
+        $registrationRole = Role::query()->first();
+        $this->setSettings(['registration-enabled' => 'true', 'registration-role' => $registrationRole->id]);
+        /** @var User $user */
         $user = User::factory()->make();
 
         // Test form and ensure user is created
@@ -57,7 +61,12 @@ class AuthTest extends TestCase
         $resp = $this->get('/');
         $resp->assertOk();
         $resp->assertSee($user->name);
+
         $this->assertDatabaseHas('users', ['name' => $user->name, 'email' => $user->email]);
+
+        $user = User::query()->where('email', '=', $user->email)->first();
+        $this->assertEquals(1, $user->roles()->count());
+        $this->assertEquals($registrationRole->id, $user->roles()->first()->id);
     }
 
     public function test_empty_registration_redirects_back_with_errors()
@@ -189,6 +198,14 @@ class AuthTest extends TestCase
         $this->assertNull(auth()->user());
     }
 
+    public function test_registration_role_unset_by_default()
+    {
+        $this->assertFalse(setting('registration-role'));
+
+        $resp = $this->asAdmin()->get('/settings');
+        $resp->assertElementContains('select[name="setting-registration-role"] option[value="0"][selected]', '-- None --');
+    }
+
     public function test_logout()
     {
         $this->asAdmin()->get('/')->assertOk();