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

- Wrap the user creation process in a transaction
- Add test
This commit is contained in:
julesdevops 2022-01-19 20:46:38 +01:00
parent 2aace16704
commit c9beacbfbf
2 changed files with 60 additions and 11 deletions

View File

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

View File

@ -3,11 +3,14 @@
namespace Tests\User; namespace Tests\User;
use BookStack\Actions\ActivityType; use BookStack\Actions\ActivityType;
use BookStack\Auth\Access\UserInviteService;
use BookStack\Auth\Role; use BookStack\Auth\Role;
use BookStack\Auth\User; use BookStack\Auth\User;
use BookStack\Entities\Models\Page; use BookStack\Entities\Models\Page;
use Illuminate\Support\Facades\Hash; use Illuminate\Support\Facades\Hash;
use Illuminate\Support\Str; use Illuminate\Support\Str;
use Mockery\MockInterface;
use RuntimeException;
use Tests\TestCase; use Tests\TestCase;
class UserManagementTest extends TestCase class UserManagementTest extends TestCase
@ -179,4 +182,46 @@ class UserManagementTest extends TestCase
$resp = $this->followRedirects($resp); $resp = $this->followRedirects($resp);
$resp->assertSee('cannot delete the guest user'); $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']);
}
} }