From d12e8ec92369959ce2f3f43c4e047ab28e735538 Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Sun, 29 Sep 2024 16:41:18 +0100
Subject: [PATCH] Users: Improved user response for failed invite sending
Added specific handling to show relevant error message when user
creation fails due to invite sending errors, while also returning user
to the form with previous input.
Includes test to cover.
For #5195
---
app/Access/UserInviteException.php | 10 ++++++++
app/Access/UserInviteService.php | 8 ++++++-
app/Users/Controllers/UserController.php | 15 +++++++++---
app/Users/UserRepo.php | 2 ++
lang/en/errors.php | 1 +
tests/User/UserManagementTest.php | 29 ++++++++++++++++++++----
6 files changed, 57 insertions(+), 8 deletions(-)
create mode 100644 app/Access/UserInviteException.php
diff --git a/app/Access/UserInviteException.php b/app/Access/UserInviteException.php
new file mode 100644
index 000000000..70e7a7872
--- /dev/null
+++ b/app/Access/UserInviteException.php
@@ -0,0 +1,10 @@
+<?php
+
+namespace BookStack\Access;
+
+use Exception;
+
+class UserInviteException extends Exception
+{
+ //
+}
diff --git a/app/Access/UserInviteService.php b/app/Access/UserInviteService.php
index 7d955f385..0b24eb0d9 100644
--- a/app/Access/UserInviteService.php
+++ b/app/Access/UserInviteService.php
@@ -13,11 +13,17 @@ class UserInviteService extends UserTokenService
/**
* Send an invitation to a user to sign into BookStack
* Removes existing invitation tokens.
+ * @throws UserInviteException
*/
public function sendInvitation(User $user)
{
$this->deleteByUser($user);
$token = $this->createTokenForUser($user);
- $user->notify(new UserInviteNotification($token));
+
+ try {
+ $user->notify(new UserInviteNotification($token));
+ } catch (\Exception $exception) {
+ throw new UserInviteException($exception->getMessage(), $exception->getCode(), $exception);
+ }
}
}
diff --git a/app/Users/Controllers/UserController.php b/app/Users/Controllers/UserController.php
index 185d6101c..00bbe6118 100644
--- a/app/Users/Controllers/UserController.php
+++ b/app/Users/Controllers/UserController.php
@@ -3,6 +3,7 @@
namespace BookStack\Users\Controllers;
use BookStack\Access\SocialDriverManager;
+use BookStack\Access\UserInviteException;
use BookStack\Exceptions\ImageUploadException;
use BookStack\Exceptions\UserUpdateException;
use BookStack\Http\Controller;
@@ -14,6 +15,7 @@ use BookStack\Util\SimpleListOptions;
use Exception;
use Illuminate\Http\Request;
use Illuminate\Support\Facades\DB;
+use Illuminate\Support\Facades\Log;
use Illuminate\Validation\Rules\Password;
use Illuminate\Validation\ValidationException;
@@ -91,9 +93,16 @@ class UserController extends Controller
$validated = $this->validate($request, array_filter($validationRules));
- DB::transaction(function () use ($validated, $sendInvite) {
- $this->userRepo->create($validated, $sendInvite);
- });
+ try {
+ DB::transaction(function () use ($validated, $sendInvite) {
+ $this->userRepo->create($validated, $sendInvite);
+ dd('post-create');
+ });
+ } catch (UserInviteException $e) {
+ Log::error("Failed to send user invite with error: {$e->getMessage()}");
+ $this->showErrorNotification(trans('errors.users_could_not_send_invite'));
+ return redirect('/settings/users/create')->withInput();
+ }
return redirect('/settings/users');
}
diff --git a/app/Users/UserRepo.php b/app/Users/UserRepo.php
index 32e23ecde..5c8ace8fa 100644
--- a/app/Users/UserRepo.php
+++ b/app/Users/UserRepo.php
@@ -2,6 +2,7 @@
namespace BookStack\Users;
+use BookStack\Access\UserInviteException;
use BookStack\Access\UserInviteService;
use BookStack\Activity\ActivityType;
use BookStack\Entities\EntityProvider;
@@ -83,6 +84,7 @@ class UserRepo
* As per "createWithoutActivity" but records a "create" activity.
*
* @param array{name: string, email: string, password: ?string, external_auth_id: ?string, language: ?string, roles: ?array} $data
+ * @throws UserInviteException
*/
public function create(array $data, bool $sendInvite = false): User
{
diff --git a/lang/en/errors.php b/lang/en/errors.php
index 752eb5672..9c40aa9ed 100644
--- a/lang/en/errors.php
+++ b/lang/en/errors.php
@@ -78,6 +78,7 @@ return [
// Users
'users_cannot_delete_only_admin' => 'You cannot delete the only admin',
'users_cannot_delete_guest' => 'You cannot delete the guest user',
+ 'users_could_not_send_invite' => 'Could not create user since invite email failed to send',
// Roles
'role_cannot_be_edited' => 'This role cannot be edited',
diff --git a/tests/User/UserManagementTest.php b/tests/User/UserManagementTest.php
index 93d35f5d0..8fe855afa 100644
--- a/tests/User/UserManagementTest.php
+++ b/tests/User/UserManagementTest.php
@@ -2,6 +2,7 @@
namespace Tests\User;
+use BookStack\Access\UserInviteException;
use BookStack\Access\UserInviteService;
use BookStack\Activity\ActivityType;
use BookStack\Uploads\Image;
@@ -229,7 +230,7 @@ class UserManagementTest extends TestCase
// Simulate an invitation sending failure
$this->mock(UserInviteService::class, function (MockInterface $mock) {
- $mock->shouldReceive('sendInvitation')->once()->andThrow(RuntimeException::class);
+ $mock->shouldReceive('sendInvitation')->once()->andThrow(UserInviteException::class);
});
$this->asAdmin()->post('/settings/users/create', [
@@ -247,22 +248,42 @@ class UserManagementTest extends TestCase
{
/** @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);
+ $mock->shouldReceive('sendInvitation')->once()->andThrow(UserInviteException::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']);
}
+ public function test_return_to_form_with_warning_if_the_invitation_sending_fails()
+ {
+ $logger = $this->withTestLogger();
+ /** @var User $user */
+ $user = User::factory()->make();
+
+ $this->mock(UserInviteService::class, function (MockInterface $mock) {
+ $mock->shouldReceive('sendInvitation')->once()->andThrow(UserInviteException::class);
+ });
+
+ $resp = $this->asAdmin()->post('/settings/users/create', [
+ 'name' => $user->name,
+ 'email' => $user->email,
+ 'send_invite' => 'true',
+ ]);
+
+ $resp->assertRedirect('/settings/users/create');
+ $this->assertSessionError('Could not create user since invite email failed to send');
+ $this->assertEquals($user->email, session()->getOldInput('email'));
+ $this->assertTrue($logger->hasErrorThatContains('Failed to send user invite with error:'));
+ }
+
public function test_user_create_update_fails_if_locale_is_invalid()
{
$user = $this->users->editor();