diff --git a/app/Http/Controllers/UserController.php b/app/Http/Controllers/UserController.php index a78f921f2..4085e151c 100644 --- 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); - } + DB::transaction(function () use ($user, $sendInvite, $request) { + $user->save(); - if ($request->filled('roles')) { - $roles = $request->get('roles'); - $this->userRepo->setUserRoles($user, $roles); - } + if ($sendInvite) { + $this->inviteService->sendInvitation($user); + } - $this->userRepo->downloadAndAssignUserAvatar($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 94970df4f..806e35ad4 100644 --- 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']); + } }