BookStackApp · ssddanbrown · Feb 8, 2022 · May 5, 2021 · May 6, 2021 · Feb 3, 2022
diff --git a/app/Api/ApiDocsGenerator.php b/app/Api/ApiDocsGenerator.php
@@ -3,11 +3,13 @@
 namespace BookStack\Api;
 
 use BookStack\Http\Controllers\Api\ApiController;
+use Exception;
 use Illuminate\Contracts\Container\BindingResolutionException;
 use Illuminate\Support\Collection;
 use Illuminate\Support\Facades\Cache;
 use Illuminate\Support\Facades\Route;
 use Illuminate\Support\Str;
+use Illuminate\Validation\Rules\Password;
 use ReflectionClass;
 use ReflectionException;
 use ReflectionMethod;
@@ -100,11 +102,36 @@ protected function getBodyParamsFromClass(string $className, string $methodName)
             $this->controllerClasses[$className] = $class;
         }
 
-        $rules = $class->getValdationRules()[$methodName] ?? [];
+        $rules = collect($class->getValidationRules()[$methodName] ?? [])->map(function($validations) {
+            return array_map(function($validation) {
+                return $this->getValidationAsString($validation);
+            }, $validations);
+        })->toArray();
 
         return empty($rules) ? null : $rules;
     }
 
+    /**
+     * Convert the given validation message to a readable string.
+     */
+    protected function getValidationAsString($validation): string
+    {
+        if (is_string($validation)) {
+            return $validation;
+        }
+
+        if (is_object($validation) && method_exists($validation, '__toString')) {
+            return strval($validation);
+        }
+
+        if ($validation instanceof Password) {
+            return 'min:8';
+        }
+
+        $class = get_class($validation);
+        throw new Exception("Cannot provide string representation of rule for class: {$class}");
+    }
+
     /**
      * Parse out the description text from a class method comment.
      */

diff --git a/app/Api/ListingResponseBuilder.php b/app/Api/ListingResponseBuilder.php
@@ -2,8 +2,10 @@
 
 namespace BookStack\Api;
 
+use BookStack\Model;
 use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Database\Eloquent\Collection;
+use Illuminate\Http\JsonResponse;
 use Illuminate\Http\Request;
 
 class ListingResponseBuilder
@@ -12,6 +14,11 @@ class ListingResponseBuilder
     protected $request;
     protected $fields;
 
+    /**
+     * @var array<callable>
+     */
+    protected $resultModifiers = [];
+
     protected $filterOperators = [
         'eq'   => '=',
         'ne'   => '!=',
@@ -24,6 +31,7 @@ class ListingResponseBuilder
 
     /**
      * ListingResponseBuilder constructor.
+     * The given fields will be forced visible within the model results.
      */
     public function __construct(Builder $query, Request $request, array $fields)
     {
@@ -35,12 +43,16 @@ public function __construct(Builder $query, Request $request, array $fields)
     /**
      * Get the response from this builder.
      */
-    public function toResponse()
+    public function toResponse(): JsonResponse
     {
         $filteredQuery = $this->filterQuery($this->query);
 
         $total = $filteredQuery->count();
-        $data = $this->fetchData($filteredQuery);
+        $data = $this->fetchData($filteredQuery)->each(function($model) {
+            foreach ($this->resultModifiers as $modifier) {
+                $modifier($model);
+            }
+        });
 
         return response()->json([
             'data'  => $data,
@@ -49,7 +61,16 @@ public function toResponse()
     }
 
     /**
-     * Fetch the data to return in the response.
+     * Add a callback to modify each element of the results
+     * @param (callable(Model)) $modifier
+     */
+    public function modifyResults($modifier): void
+    {
+        $this->resultModifiers[] = $modifier;
+    }
+
+    /**
+     * Fetch the data to return within the response.
      */
     protected function fetchData(Builder $query): Collection
     {

diff --git a/app/Auth/Access/Guards/LdapSessionGuard.php b/app/Auth/Access/Guards/LdapSessionGuard.php
@@ -84,7 +84,7 @@ public function attempt(array $credentials = [], $remember = false)
             try {
                 $user = $this->createNewFromLdapAndCreds($userDetails, $credentials);
             } catch (UserRegistrationException $exception) {
-                throw new LoginAttemptException($exception->message);
+                throw new LoginAttemptException($exception->getMessage());
             }
         }
 

diff --git a/app/Auth/Access/RegistrationService.php b/app/Auth/Access/RegistrationService.php
@@ -96,7 +96,8 @@ public function registerUser(array $userData, ?SocialAccount $socialAccount = nu
         }
 
         // Create the user
-        $newUser = $this->userRepo->registerNew($userData, $emailConfirmed);
+        $newUser = $this->userRepo->createWithoutActivity($userData, $emailConfirmed);
+        $newUser->attachDefaultRole();
 
         // Assign social account if given
         if ($socialAccount) {

diff --git a/app/Auth/Role.php b/app/Auth/Role.php
@@ -28,6 +28,8 @@ class Role extends Model implements Loggable
 
     protected $fillable = ['display_name', 'description', 'external_auth_id'];
 
+    protected $hidden = ['pivot'];
+
     /**
      * The roles that belong to the role.
      */

diff --git a/app/Auth/User.php b/app/Auth/User.php
@@ -72,7 +72,7 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon
      */
     protected $hidden = [
         'password', 'remember_token', 'system_name', 'email_confirmed', 'external_auth_id', 'email',
-        'created_at', 'updated_at', 'image_id',
+        'created_at', 'updated_at', 'image_id', 'roles', 'avatar', 'user_id',
     ];
 
     /**

diff --git a/app/Auth/UserRepo.php b/app/Auth/UserRepo.php
@@ -2,30 +2,37 @@
 
 namespace BookStack\Auth;
 
+use BookStack\Actions\ActivityType;
+use BookStack\Auth\Access\UserInviteService;
 use BookStack\Entities\EntityProvider;
 use BookStack\Entities\Models\Book;
 use BookStack\Entities\Models\Bookshelf;
 use BookStack\Entities\Models\Chapter;
 use BookStack\Entities\Models\Page;
 use BookStack\Exceptions\NotFoundException;
+use BookStack\Exceptions\NotifyException;
 use BookStack\Exceptions\UserUpdateException;
+use BookStack\Facades\Activity;
 use BookStack\Uploads\UserAvatars;
 use Exception;
 use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Database\Eloquent\Collection;
 use Illuminate\Pagination\LengthAwarePaginator;
 use Illuminate\Support\Facades\Log;
+use Illuminate\Support\Str;
 
 class UserRepo
 {
     protected $userAvatar;
+    protected $inviteService;
 
     /**
      * UserRepo constructor.
      */
-    public function __construct(UserAvatars $userAvatar)
+    public function __construct(UserAvatars $userAvatar, UserInviteService $inviteService)
     {
         $this->userAvatar = $userAvatar;
+        $this->inviteService = $inviteService;
     }
 
     /**
@@ -53,11 +60,13 @@ public function getBySlug(string $slug): User
     }
 
     /**
-     * Get all the users with their permissions.
+     * Get all users as Builder for API
      */
-    public function getAllUsers(): Collection
+    public function getApiUsersBuilder(): Builder
     {
-        return User::query()->with('roles', 'avatar')->orderBy('name', 'asc')->get();
+        return User::query()->select(['*'])
+            ->scopes('withLastActivityAt')
+            ->with(['avatar']);
     }
 
     /**
@@ -87,18 +96,6 @@ public function getAllUsersPaginatedAndSorted(int $count, array $sortData): Leng
         return $query->paginate($count);
     }
 
-    /**
-     * Creates a new user and attaches a role to them.
-     */
-    public function registerNew(array $data, bool $emailConfirmed = false): User
-    {
-        $user = $this->create($data, $emailConfirmed);
-        $user->attachDefaultRole();
-        $this->downloadAndAssignUserAvatar($user);
-
-        return $user;
-    }
-
     /**
      * Assign a user to a system-level role.
      *
@@ -161,23 +158,85 @@ protected function demotingLastAdmin(User $user, array $newRoles): bool
     }
 
     /**
-     * Create a new basic instance of user.
+     * Create a new basic instance of user with the given pre-validated data.
+     * @param array{name: string, email: string, password: ?string, external_auth_id: ?string, language: ?string, roles: ?array} $data
      */
-    public function create(array $data, bool $emailConfirmed = false): User
+    public function createWithoutActivity(array $data, bool $emailConfirmed = false): User
     {
-        $details = [
-            'name'             => $data['name'],
-            'email'            => $data['email'],
-            'password'         => bcrypt($data['password']),
-            'email_confirmed'  => $emailConfirmed,
-            'external_auth_id' => $data['external_auth_id'] ?? '',
-        ];
-
         $user = new User();
-        $user->forceFill($details);
+        $user->name = $data['name'];
+        $user->email = $data['email'];
+        $user->password = bcrypt(empty($data['password']) ? Str::random(32) : $data['password']);
+        $user->email_confirmed = $emailConfirmed;
+        $user->external_auth_id = $data['external_auth_id'] ?? '';
+
         $user->refreshSlug();
         $user->save();
 
+        if (!empty($data['language'])) {
+            setting()->putUser($user, 'language', $data['language']);
+        }
+
+        if (isset($data['roles'])) {
+            $this->setUserRoles($user, $data['roles']);
+        }
+
+        $this->downloadAndAssignUserAvatar($user);
+
+        return $user;
+    }
+
+    /**
+     * 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
+     */
+    public function create(array $data, bool $sendInvite = false): User
+    {
+        $user = $this->createWithoutActivity($data, true);
+
+        if ($sendInvite) {
+            $this->inviteService->sendInvitation($user);
+        }
+
+        Activity::add(ActivityType::USER_CREATE, $user);
+        return $user;
+    }
+
+    /**
+     * Update the given user with the given data.
+     * @param array{name: ?string, email: ?string, external_auth_id: ?string, password: ?string, roles: ?array<int>, language: ?string} $data
+     * @throws UserUpdateException
+     */
+    public function update(User $user, array $data, bool $manageUsersAllowed): User
+    {
+        if (!empty($data['name'])) {
+            $user->name = $data['name'];
+            $user->refreshSlug();
+        }
+
+        if (!empty($data['email']) && $manageUsersAllowed) {
+            $user->email = $data['email'];
+        }
+
+        if (!empty($data['external_auth_id']) && $manageUsersAllowed) {
+            $user->external_auth_id = $data['external_auth_id'];
+        }
+
+        if (isset($data['roles']) && $manageUsersAllowed) {
+            $this->setUserRoles($user, $data['roles']);
+        }
+
+        if (!empty($data['password'])) {
+            $user->password = bcrypt($data['password']);
+        }
+
+        if (!empty($data['language'])) {
+            setting()->putUser($user, 'language', $data['language']);
+        }
+
+        $user->save();
+        Activity::add(ActivityType::USER_UPDATE, $user);
+
         return $user;
     }
 
@@ -188,6 +247,8 @@ public function create(array $data, bool $emailConfirmed = false): User
      */
     public function destroy(User $user, ?int $newOwnerId = null)
     {
+        $this->ensureDeletable($user);
+
         $user->socialAccounts()->delete();
         $user->apiTokens()->delete();
         $user->favourites()->delete();
@@ -203,6 +264,22 @@ public function destroy(User $user, ?int $newOwnerId = null)
                 $this->migrateOwnership($user, $newOwner);
             }
         }
+
+        Activity::add(ActivityType::USER_DELETE, $user);
+    }
+
+    /**
+     * @throws NotifyException
+     */
+    protected function ensureDeletable(User $user): void
+    {
+        if ($this->isOnlyAdmin($user)) {
+            throw new NotifyException(trans('errors.users_cannot_delete_only_admin'), $user->getEditUrl());
+        }
+
+        if ($user->system_name === 'public') {
+            throw new NotifyException(trans('errors.users_cannot_delete_guest'), $user->getEditUrl());
+        }
     }
 
     /**
@@ -230,10 +307,10 @@ public function getRecentlyCreated(User $user, int $count = 20): array
         };
 
         return [
-            'pages'    => $query(Page::visible()->where('draft', '=', false)),
+            'pages' => $query(Page::visible()->where('draft', '=', false)),
             'chapters' => $query(Chapter::visible()),
-            'books'    => $query(Book::visible()),
-            'shelves'  => $query(Bookshelf::visible()),
+            'books' => $query(Book::visible()),
+            'shelves' => $query(Bookshelf::visible()),
         ];
     }
 
@@ -245,10 +322,10 @@ public function getAssetCounts(User $user): array
         $createdBy = ['created_by' => $user->id];
 
         return [
-            'pages'       => Page::visible()->where($createdBy)->count(),
-            'chapters'    => Chapter::visible()->where($createdBy)->count(),
-            'books'       => Book::visible()->where($createdBy)->count(),
-            'shelves'     => Bookshelf::visible()->where($createdBy)->count(),
+            'pages' => Page::visible()->where($createdBy)->count(),
+            'chapters' => Chapter::visible()->where($createdBy)->count(),
+            'books' => Book::visible()->where($createdBy)->count(),
+            'shelves' => Bookshelf::visible()->where($createdBy)->count(),
         ];
     }
-Original file line number
+Diff line change
@@ Expand Up / @@ -28,6 +28,8 @@ class Role extends Model implements Loggable @@
         protected $fillable = ['display_name', 'description', 'external_auth_id'];
+        protected $hidden = ['pivot'];
         /**
          * The roles that belong to the role.
          */
@@ Expand Down @@