fix(User Creation): do not persist the user if invitation fails

- Wrap the user creation process in a transaction
- Add test

diff --git a/app/Http/Controllers/UserController.php b/app/Http/Controllers/UserController.php
index a78f921f21e2ea187b906be6e3b4783d77d70e23..4085e151c7ca7a69198245dd6e6680da51ae1a47 100644 (file)
--- a/app/Http/Controllers/UserController.php
+++ b/app/Http/Controllers/UserController.php
@@ -12,6 +12,7 @@ use BookStack\Exceptions\UserUpdateException;
 use BookStack\Uploads\ImageRepo;
 use Exception;
 use Illuminate\Http\Request;
+use Illuminate\Support\Facades\DB;
 use Illuminate\Support\Str;
 use Illuminate\Validation\Rules\Password;
 use Illuminate\Validation\ValidationException;
@@ -99,20 +100,23 @@ class UserController extends Controller
         }
 
         $user->refreshSlug();
-        $user->save();
-
-        if ($sendInvite) {
-            $this->inviteService->sendInvitation($user);
-        }
 
-        if ($request->filled('roles')) {
-            $roles = $request->get('roles');
-            $this->userRepo->setUserRoles($user, $roles);
-        }
+        DB::transaction(function () use ($user, $sendInvite, $request) {
+            $user->save();
 
-        $this->userRepo->downloadAndAssignUserAvatar($user);
+            if ($sendInvite) {
+                $this->inviteService->sendInvitation($user);
+            }
 
-        $this->logActivity(ActivityType::USER_CREATE, $user);
+            if ($request->filled('roles')) {
+                $roles = $request->get('roles');
+                $this->userRepo->setUserRoles($user, $roles);
+            }
+    
+            $this->userRepo->downloadAndAssignUserAvatar($user);
+    
+            $this->logActivity(ActivityType::USER_CREATE, $user);
+        });
 
         return redirect('/settings/users');
     }

diff --git a/tests/User/UserManagementTest.php b/tests/User/UserManagementTest.php
index 94970df4fe5b800eafb39ca6a3617ed6b6a7e6f2..806e35ad4cf9a0751bbaa2587f6f8c93faba24b6 100644 (file)
--- a/tests/User/UserManagementTest.php
+++ b/tests/User/UserManagementTest.php
@@ -3,11 +3,14 @@
 namespace Tests\User;
 
 use BookStack\Actions\ActivityType;
+use BookStack\Auth\Access\UserInviteService;
 use BookStack\Auth\Role;
 use BookStack\Auth\User;
 use BookStack\Entities\Models\Page;
 use Illuminate\Support\Facades\Hash;
 use Illuminate\Support\Str;
+use Mockery\MockInterface;
+use RuntimeException;
 use Tests\TestCase;
 
 class UserManagementTest extends TestCase
@@ -179,4 +182,46 @@ class UserManagementTest extends TestCase
         $resp = $this->followRedirects($resp);
         $resp->assertSee('cannot delete the guest user');
     }
+
+    public function test_user_creation_is_not_performed_if_the_invitation_sending_fails()
+    {
+        /** @var User $user */
+        $user = User::factory()->make();
+        $adminRole = Role::getRole('admin');
+
+        // Simulate an invitation sending failure
+        $this->mock(UserInviteService::class, function (MockInterface $mock) {
+            $mock->shouldReceive('sendInvitation')->once()->andThrow(RuntimeException::class);
+        });
+
+        $this->asAdmin()->post('/settings/users/create', [
+            'name'                          => $user->name,
+            'email'                         => $user->email,
+            'send_invite'                   => 'true',
+            'roles[' . $adminRole->id . ']' => 'true',
+        ]);
+
+        // Since the invitation failed, the user should not exist in the database
+        $this->assertDatabaseMissing('users', $user->only('name', 'email'));
+    }
+
+    public function test_user_create_activity_is_not_persisted_if_the_invitation_sending_fails()
+    {
+        /** @var User $user */
+        $user = User::factory()->make();
+        $adminRole = Role::getRole('admin');
+
+        $this->mock(UserInviteService::class, function (MockInterface $mock) {
+            $mock->shouldReceive('sendInvitation')->once()->andThrow(RuntimeException::class);
+        });
+
+        $this->asAdmin()->post('/settings/users/create', [
+            'name'                          => $user->name,
+            'email'                         => $user->email,
+            'send_invite'                   => 'true',
+            'roles[' . $adminRole->id . ']' => 'true',
+        ]);
+
+        $this->assertDatabaseMissing('activities', ['type' => 'USER_CREATE']);
+    }
 }