From 1e34954554d672446693bcc74bf7f9aae1936802 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Tue, 2 Sep 2025 11:10:47 +0100 Subject: [PATCH 1/5] Maintenance: Continued work towards PHPstan level 2 Updated html description code to be behind a proper interface. Set new convention for mode traits/interfaces. --- app/Entities/Models/Book.php | 5 +-- app/Entities/Models/Bookshelf.php | 4 +-- app/Entities/Models/Chapter.php | 4 +-- ...CoverImage.php => CoverImageInterface.php} | 2 +- .../{Deletable.php => DeletableInterface.php} | 2 +- app/Entities/Models/Deletion.php | 2 +- app/Entities/Models/Entity.php | 2 +- app/Entities/Models/HasHtmlDescription.php | 21 ----------- .../Models/HtmlDescriptionInterface.php | 17 +++++++++ app/Entities/Models/HtmlDescriptionTrait.php | 35 +++++++++++++++++++ app/Entities/Queries/PageQueries.php | 3 ++ app/Entities/Repos/BaseRepo.php | 20 +++++------ app/Entities/Tools/Cloner.php | 4 +-- app/Entities/Tools/PageIncludeParser.php | 7 ++-- app/Entities/Tools/TrashCan.php | 4 +-- app/References/ReferenceUpdater.php | 13 ++++--- app/Sorting/SortRuleController.php | 2 +- .../Controllers/ImageGalleryApiController.php | 4 +-- 18 files changed, 94 insertions(+), 57 deletions(-) rename app/Entities/Models/{HasCoverImage.php => CoverImageInterface.php} (92%) rename app/Entities/Models/{Deletable.php => DeletableInterface.php} (90%) delete mode 100644 app/Entities/Models/HasHtmlDescription.php create mode 100644 app/Entities/Models/HtmlDescriptionInterface.php create mode 100644 app/Entities/Models/HtmlDescriptionTrait.php diff --git a/app/Entities/Models/Book.php b/app/Entities/Models/Book.php index ede4fc7d5..88e3b85ba 100644 --- a/app/Entities/Models/Book.php +++ b/app/Entities/Models/Book.php @@ -26,10 +26,10 @@ use Illuminate\Support\Collection; * @property ?Page $defaultTemplate * @property ?SortRule $sortRule */ -class Book extends Entity implements HasCoverImage +class Book extends Entity implements CoverImageInterface, HtmlDescriptionInterface { use HasFactory; - use HasHtmlDescription; + use HtmlDescriptionTrait; public float $searchFactor = 1.2; @@ -111,6 +111,7 @@ class Book extends Entity implements HasCoverImage /** * Get all chapters within this book. + * @return HasMany */ public function chapters(): HasMany { diff --git a/app/Entities/Models/Bookshelf.php b/app/Entities/Models/Bookshelf.php index 9ffa0ea9c..5b403d9c0 100644 --- a/app/Entities/Models/Bookshelf.php +++ b/app/Entities/Models/Bookshelf.php @@ -8,10 +8,10 @@ use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Relations\BelongsTo; use Illuminate\Database\Eloquent\Relations\BelongsToMany; -class Bookshelf extends Entity implements HasCoverImage +class Bookshelf extends Entity implements CoverImageInterface, HtmlDescriptionInterface { use HasFactory; - use HasHtmlDescription; + use HtmlDescriptionTrait; protected $table = 'bookshelves'; diff --git a/app/Entities/Models/Chapter.php b/app/Entities/Models/Chapter.php index 088d199da..03e819c06 100644 --- a/app/Entities/Models/Chapter.php +++ b/app/Entities/Models/Chapter.php @@ -14,10 +14,10 @@ use Illuminate\Support\Collection; * @property ?int $default_template_id * @property ?Page $defaultTemplate */ -class Chapter extends BookChild +class Chapter extends BookChild implements HtmlDescriptionInterface { use HasFactory; - use HasHtmlDescription; + use HtmlDescriptionTrait; public float $searchFactor = 1.2; diff --git a/app/Entities/Models/HasCoverImage.php b/app/Entities/Models/CoverImageInterface.php similarity index 92% rename from app/Entities/Models/HasCoverImage.php rename to app/Entities/Models/CoverImageInterface.php index f665efce6..5f781fe02 100644 --- a/app/Entities/Models/HasCoverImage.php +++ b/app/Entities/Models/CoverImageInterface.php @@ -4,7 +4,7 @@ namespace BookStack\Entities\Models; use Illuminate\Database\Eloquent\Relations\BelongsTo; -interface HasCoverImage +interface CoverImageInterface { /** * Get the cover image for this item. diff --git a/app/Entities/Models/Deletable.php b/app/Entities/Models/DeletableInterface.php similarity index 90% rename from app/Entities/Models/Deletable.php rename to app/Entities/Models/DeletableInterface.php index a2c7fad81..f771d9c69 100644 --- a/app/Entities/Models/Deletable.php +++ b/app/Entities/Models/DeletableInterface.php @@ -8,7 +8,7 @@ use Illuminate\Database\Eloquent\Relations\MorphMany; * A model that can be deleted in a manner that deletions * are tracked to be part of the recycle bin system. */ -interface Deletable +interface DeletableInterface { public function deletions(): MorphMany; } diff --git a/app/Entities/Models/Deletion.php b/app/Entities/Models/Deletion.php index a73437c94..55589f61e 100644 --- a/app/Entities/Models/Deletion.php +++ b/app/Entities/Models/Deletion.php @@ -13,7 +13,7 @@ use Illuminate\Database\Eloquent\Relations\MorphTo; * @property int $deleted_by * @property string $deletable_type * @property int $deletable_id - * @property Deletable $deletable + * @property DeletableInterface $deletable */ class Deletion extends Model implements Loggable { diff --git a/app/Entities/Models/Entity.php b/app/Entities/Models/Entity.php index 1ef4e618d..46b29f93b 100644 --- a/app/Entities/Models/Entity.php +++ b/app/Entities/Models/Entity.php @@ -49,7 +49,7 @@ use Illuminate\Database\Eloquent\SoftDeletes; * @method static Builder withLastView() * @method static Builder withViewCount() */ -abstract class Entity extends Model implements Sluggable, Favouritable, Viewable, Deletable, Loggable +abstract class Entity extends Model implements Sluggable, Favouritable, Viewable, DeletableInterface, Loggable { use SoftDeletes; use HasCreatorAndUpdater; diff --git a/app/Entities/Models/HasHtmlDescription.php b/app/Entities/Models/HasHtmlDescription.php deleted file mode 100644 index c9f08616d..000000000 --- a/app/Entities/Models/HasHtmlDescription.php +++ /dev/null @@ -1,21 +0,0 @@ -description_html ?: '

' . nl2br(e($this->description)) . '

'; - return HtmlContentFilter::removeScriptsFromHtmlString($html); - } -} diff --git a/app/Entities/Models/HtmlDescriptionInterface.php b/app/Entities/Models/HtmlDescriptionInterface.php new file mode 100644 index 000000000..ffe7f0c5f --- /dev/null +++ b/app/Entities/Models/HtmlDescriptionInterface.php @@ -0,0 +1,17 @@ +description_html ?: '

' . nl2br(e($this->description)) . '

'; + if ($raw) { + return $html; + } + + return HtmlContentFilter::removeScriptsFromHtmlString($html); + } + + public function setDescriptionHtml(string $html, string|null $plaintext = null): void + { + $this->description_html = $html; + + if ($plaintext !== null) { + $this->description = $plaintext; + } + + if (empty($html) && !empty($plaintext)) { + $this->description_html = $this->descriptionHtml(); + } + } +} diff --git a/app/Entities/Queries/PageQueries.php b/app/Entities/Queries/PageQueries.php index 06298f470..f821ee86a 100644 --- a/app/Entities/Queries/PageQueries.php +++ b/app/Entities/Queries/PageQueries.php @@ -66,6 +66,9 @@ class PageQueries implements ProvidesEntityQueries }); } + /** + * @return Builder + */ public function visibleForList(): Builder { return $this->start() diff --git a/app/Entities/Repos/BaseRepo.php b/app/Entities/Repos/BaseRepo.php index ac5a44e67..f4d05d8af 100644 --- a/app/Entities/Repos/BaseRepo.php +++ b/app/Entities/Repos/BaseRepo.php @@ -7,8 +7,9 @@ use BookStack\Entities\Models\Book; use BookStack\Entities\Models\BookChild; use BookStack\Entities\Models\Chapter; use BookStack\Entities\Models\Entity; -use BookStack\Entities\Models\HasCoverImage; -use BookStack\Entities\Models\HasHtmlDescription; +use BookStack\Entities\Models\CoverImageInterface; +use BookStack\Entities\Models\HtmlDescriptionInterface; +use BookStack\Entities\Models\HtmlDescriptionTrait; use BookStack\Entities\Queries\PageQueries; use BookStack\Exceptions\ImageUploadException; use BookStack\References\ReferenceStore; @@ -88,7 +89,7 @@ class BaseRepo /** * Update the given items' cover image, or clear it. * - * @param Entity&HasCoverImage $entity + * @param Entity&CoverImageInterface $entity * * @throws ImageUploadException * @throws \Exception @@ -150,18 +151,17 @@ class BaseRepo protected function updateDescription(Entity $entity, array $input): void { - if (!in_array(HasHtmlDescription::class, class_uses($entity))) { + if (!($entity instanceof HtmlDescriptionInterface)) { return; } - /** @var HasHtmlDescription $entity */ if (isset($input['description_html'])) { - $entity->description_html = HtmlDescriptionFilter::filterFromString($input['description_html']); - $entity->description = html_entity_decode(strip_tags($input['description_html'])); + $entity->setDescriptionHtml( + HtmlDescriptionFilter::filterFromString($input['description_html']), + html_entity_decode(strip_tags($input['description_html'])) + ); } else if (isset($input['description'])) { - $entity->description = $input['description']; - $entity->description_html = ''; - $entity->description_html = $entity->descriptionHtml(); + $entity->setDescriptionHtml('', $input['description']); } } } diff --git a/app/Entities/Tools/Cloner.php b/app/Entities/Tools/Cloner.php index 2be6083e3..87aa770c0 100644 --- a/app/Entities/Tools/Cloner.php +++ b/app/Entities/Tools/Cloner.php @@ -7,7 +7,7 @@ use BookStack\Entities\Models\Book; use BookStack\Entities\Models\Bookshelf; use BookStack\Entities\Models\Chapter; use BookStack\Entities\Models\Entity; -use BookStack\Entities\Models\HasCoverImage; +use BookStack\Entities\Models\CoverImageInterface; use BookStack\Entities\Models\Page; use BookStack\Entities\Repos\BookRepo; use BookStack\Entities\Repos\ChapterRepo; @@ -105,7 +105,7 @@ class Cloner $inputData['tags'] = $this->entityTagsToInputArray($entity); // Add a cover to the data if existing on the original entity - if ($entity instanceof HasCoverImage) { + if ($entity instanceof CoverImageInterface) { $cover = $entity->cover()->first(); if ($cover) { $inputData['image'] = $this->imageToUploadedFile($cover); diff --git a/app/Entities/Tools/PageIncludeParser.php b/app/Entities/Tools/PageIncludeParser.php index e0b89f158..329a7633f 100644 --- a/app/Entities/Tools/PageIncludeParser.php +++ b/app/Entities/Tools/PageIncludeParser.php @@ -7,7 +7,6 @@ use Closure; use DOMDocument; use DOMElement; use DOMNode; -use DOMText; class PageIncludeParser { @@ -159,7 +158,7 @@ class PageIncludeParser /** * Splits the given $parentNode at the location of the $domNode within it. - * Attempts replicate the original $parentNode, moving some of their parent + * Attempts to replicate the original $parentNode, moving some of their parent * children in where needed, before adding the $domNode between. */ protected function splitNodeAtChildNode(DOMElement $parentNode, DOMNode $domNode): void @@ -171,6 +170,10 @@ class PageIncludeParser } $parentClone = $parentNode->cloneNode(); + if (!($parentClone instanceof DOMElement)) { + return; + } + $parentNode->parentNode->insertBefore($parentClone, $parentNode); $parentClone->removeAttribute('id'); diff --git a/app/Entities/Tools/TrashCan.php b/app/Entities/Tools/TrashCan.php index 5e8a93719..d457d4f48 100644 --- a/app/Entities/Tools/TrashCan.php +++ b/app/Entities/Tools/TrashCan.php @@ -8,7 +8,7 @@ use BookStack\Entities\Models\Bookshelf; use BookStack\Entities\Models\Chapter; use BookStack\Entities\Models\Deletion; use BookStack\Entities\Models\Entity; -use BookStack\Entities\Models\HasCoverImage; +use BookStack\Entities\Models\CoverImageInterface; use BookStack\Entities\Models\Page; use BookStack\Entities\Queries\EntityQueries; use BookStack\Exceptions\NotifyException; @@ -398,7 +398,7 @@ class TrashCan $entity->referencesTo()->delete(); $entity->referencesFrom()->delete(); - if ($entity instanceof HasCoverImage && $entity->cover()->exists()) { + if ($entity instanceof CoverImageInterface && $entity->cover()->exists()) { $imageService = app()->make(ImageService::class); $imageService->destroy($entity->cover()->first()); } diff --git a/app/References/ReferenceUpdater.php b/app/References/ReferenceUpdater.php index db355f211..5f1d711e9 100644 --- a/app/References/ReferenceUpdater.php +++ b/app/References/ReferenceUpdater.php @@ -4,7 +4,8 @@ namespace BookStack\References; use BookStack\Entities\Models\Book; use BookStack\Entities\Models\Entity; -use BookStack\Entities\Models\HasHtmlDescription; +use BookStack\Entities\Models\HtmlDescriptionInterface; +use BookStack\Entities\Models\HtmlDescriptionTrait; use BookStack\Entities\Models\Page; use BookStack\Entities\Repos\RevisionRepo; use BookStack\Util\HtmlDocument; @@ -61,20 +62,18 @@ class ReferenceUpdater { if ($entity instanceof Page) { $this->updateReferencesWithinPage($entity, $oldLink, $newLink); - return; } - if (in_array(HasHtmlDescription::class, class_uses($entity))) { + if ($entity instanceof HtmlDescriptionInterface) { $this->updateReferencesWithinDescription($entity, $oldLink, $newLink); } } - protected function updateReferencesWithinDescription(Entity $entity, string $oldLink, string $newLink): void + protected function updateReferencesWithinDescription(Entity&HtmlDescriptionInterface $entity, string $oldLink, string $newLink): void { - /** @var HasHtmlDescription&Entity $entity */ $entity = (clone $entity)->refresh(); - $html = $this->updateLinksInHtml($entity->description_html ?: '', $oldLink, $newLink); - $entity->description_html = $html; + $html = $this->updateLinksInHtml($entity->descriptionHtml(true) ?: '', $oldLink, $newLink); + $entity->setDescriptionHtml($html); $entity->save(); } diff --git a/app/Sorting/SortRuleController.php b/app/Sorting/SortRuleController.php index 96b8e8ef5..a124ffa9c 100644 --- a/app/Sorting/SortRuleController.php +++ b/app/Sorting/SortRuleController.php @@ -29,7 +29,7 @@ class SortRuleController extends Controller $operations = SortRuleOperation::fromSequence($request->input('sequence')); if (count($operations) === 0) { - return redirect()->withInput()->withErrors(['sequence' => 'No operations set.']); + return redirect('/settings/sorting/rules/new')->withInput()->withErrors(['sequence' => 'No operations set.']); } $rule = new SortRule(); diff --git a/app/Uploads/Controllers/ImageGalleryApiController.php b/app/Uploads/Controllers/ImageGalleryApiController.php index 6d4657a7a..d2a750c45 100644 --- a/app/Uploads/Controllers/ImageGalleryApiController.php +++ b/app/Uploads/Controllers/ImageGalleryApiController.php @@ -146,10 +146,10 @@ class ImageGalleryApiController extends ApiController $data['content']['html'] = "
id}\">
"; $data['content']['markdown'] = $data['content']['html']; } else { - $escapedDisplayThumb = htmlentities($image->thumbs['display']); + $escapedDisplayThumb = htmlentities($image->getAttribute('thumbs')['display']); $data['content']['html'] = "\"{$escapedName}\""; $mdEscapedName = str_replace(']', '', str_replace('[', '', $image->name)); - $mdEscapedThumb = str_replace(']', '', str_replace('[', '', $image->thumbs['display'])); + $mdEscapedThumb = str_replace(']', '', str_replace('[', '', $image->getAttribute('thumbs')['display'])); $data['content']['markdown'] = "![{$mdEscapedName}]({$mdEscapedThumb})"; } From cee23de6c5350101e16e1aead59e0219e9e2771e Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Tue, 2 Sep 2025 16:02:52 +0100 Subject: [PATCH 2/5] Maintenance: Reached PHPstan level 2 Reworked some stuff around slugs to use interface in a better way. Also standardised phpdoc to use @return instead of @returns --- app/Access/Saml2Service.php | 2 +- app/Access/SocialDriverManager.php | 2 +- app/Activity/Models/Comment.php | 7 +++-- app/Activity/WatchLevels.php | 4 +-- app/App/Model.php | 2 +- .../{Sluggable.php => SluggableInterface.php} | 5 +-- .../Controllers/RecycleBinApiController.php | 19 ++++++++---- app/Entities/Models/Book.php | 3 +- app/Entities/Models/Bookshelf.php | 1 + app/Entities/Models/Chapter.php | 4 +-- app/Entities/Models/Entity.php | 31 ++++++++++++++++--- app/Entities/Queries/BookQueries.php | 3 ++ app/Entities/Queries/BookshelfQueries.php | 3 ++ app/Entities/Queries/EntityQueries.php | 2 +- .../Queries/ProvidesEntityQueries.php | 3 +- app/Entities/Repos/BaseRepo.php | 6 ++-- app/Entities/Tools/SlugGenerator.php | 12 +++---- app/Permissions/PermissionApplicator.php | 15 ++++----- app/References/CrossLinkParser.php | 2 +- app/Search/SearchIndex.php | 12 +++---- app/Sorting/BookSorter.php | 2 +- app/Theming/ThemeEvents.php | 12 +++---- app/Uploads/Attachment.php | 3 +- app/Uploads/Image.php | 3 +- app/Uploads/ImageService.php | 2 +- app/Uploads/ImageStorageDisk.php | 2 +- app/Users/Controllers/UserApiController.php | 2 +- app/Users/Models/HasCreatorAndUpdater.php | 5 +++ app/Users/Models/HasOwner.php | 19 ------------ app/Users/Models/OwnableInterface.php | 8 +++++ app/Users/Models/Role.php | 1 + app/Users/Models/User.php | 6 ++-- phpcs.xml | 2 +- phpstan.neon.dist | 2 +- 34 files changed, 118 insertions(+), 89 deletions(-) rename app/App/{Sluggable.php => SluggableInterface.php} (75%) delete mode 100644 app/Users/Models/HasOwner.php create mode 100644 app/Users/Models/OwnableInterface.php diff --git a/app/Access/Saml2Service.php b/app/Access/Saml2Service.php index bb7e9b572..106a7a229 100644 --- a/app/Access/Saml2Service.php +++ b/app/Access/Saml2Service.php @@ -51,7 +51,7 @@ class Saml2Service * Returns the SAML2 request ID, and the URL to redirect the user to. * * @throws Error - * @returns array{url: string, id: ?string} + * @return array{url: string, id: ?string} */ public function logout(User $user): array { diff --git a/app/Access/SocialDriverManager.php b/app/Access/SocialDriverManager.php index dafc0e82d..efafab560 100644 --- a/app/Access/SocialDriverManager.php +++ b/app/Access/SocialDriverManager.php @@ -55,7 +55,7 @@ class SocialDriverManager /** * Gets the names of the active social drivers, keyed by driver id. - * @returns array + * @return array */ public function getActive(): array { diff --git a/app/Activity/Models/Comment.php b/app/Activity/Models/Comment.php index 91cea4fe0..6b05a9bab 100644 --- a/app/Activity/Models/Comment.php +++ b/app/Activity/Models/Comment.php @@ -4,6 +4,8 @@ namespace BookStack\Activity\Models; use BookStack\App\Model; use BookStack\Users\Models\HasCreatorAndUpdater; +use BookStack\Users\Models\OwnableInterface; +use BookStack\Users\Models\User; use BookStack\Util\HtmlContentFilter; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Relations\BelongsTo; @@ -17,12 +19,10 @@ use Illuminate\Database\Eloquent\Relations\MorphTo; * @property int $local_id * @property string $entity_type * @property int $entity_id - * @property int $created_by - * @property int $updated_by * @property string $content_ref * @property bool $archived */ -class Comment extends Model implements Loggable +class Comment extends Model implements Loggable, OwnableInterface { use HasFactory; use HasCreatorAndUpdater; @@ -39,6 +39,7 @@ class Comment extends Model implements Loggable /** * Get the parent comment this is in reply to (if existing). + * @return BelongsTo */ public function parent(): BelongsTo { diff --git a/app/Activity/WatchLevels.php b/app/Activity/WatchLevels.php index de3c5e122..edbece2d3 100644 --- a/app/Activity/WatchLevels.php +++ b/app/Activity/WatchLevels.php @@ -36,7 +36,7 @@ class WatchLevels /** * Get all the possible values as an option_name => value array. - * @returns array + * @return array */ public static function all(): array { @@ -50,7 +50,7 @@ class WatchLevels /** * Get the watch options suited for the given entity. - * @returns array + * @return array */ public static function allSuitedFor(Entity $entity): array { diff --git a/app/App/Model.php b/app/App/Model.php index 8de5a2762..e1c7511c1 100644 --- a/app/App/Model.php +++ b/app/App/Model.php @@ -8,7 +8,7 @@ class Model extends EloquentModel { /** * Provides public access to get the raw attribute value from the model. - * Used in areas where no mutations are required but performance is critical. + * Used in areas where no mutations are required, but performance is critical. * * @return mixed */ diff --git a/app/App/Sluggable.php b/app/App/SluggableInterface.php similarity index 75% rename from app/App/Sluggable.php rename to app/App/SluggableInterface.php index f8da2e265..96af49cd3 100644 --- a/app/App/Sluggable.php +++ b/app/App/SluggableInterface.php @@ -5,11 +5,8 @@ namespace BookStack\App; /** * Assigned to models that can have slugs. * Must have the below properties. - * - * @property int $id - * @property string $name */ -interface Sluggable +interface SluggableInterface { /** * Regenerate the slug for this model. diff --git a/app/Entities/Controllers/RecycleBinApiController.php b/app/Entities/Controllers/RecycleBinApiController.php index bf22d7dcd..fdc24ddf8 100644 --- a/app/Entities/Controllers/RecycleBinApiController.php +++ b/app/Entities/Controllers/RecycleBinApiController.php @@ -6,10 +6,10 @@ use BookStack\Entities\Models\Book; use BookStack\Entities\Models\BookChild; use BookStack\Entities\Models\Chapter; use BookStack\Entities\Models\Deletion; +use BookStack\Entities\Models\Page; use BookStack\Entities\Repos\DeletionRepo; use BookStack\Http\ApiController; -use Closure; -use Illuminate\Database\Eloquent\Builder; +use Illuminate\Database\Eloquent\Relations\HasMany; class RecycleBinApiController extends ApiController { @@ -40,7 +40,7 @@ class RecycleBinApiController extends ApiController 'updated_at', 'deletable_type', 'deletable_id', - ], [Closure::fromCallable([$this, 'listFormatter'])]); + ], [$this->listFormatter(...)]); } /** @@ -72,7 +72,6 @@ class RecycleBinApiController extends ApiController protected function listFormatter(Deletion $deletion) { $deletable = $deletion->deletable; - $withTrashedQuery = fn (Builder $query) => $query->withTrashed(); if ($deletable instanceof BookChild) { $parent = $deletable->getParent(); @@ -81,11 +80,19 @@ class RecycleBinApiController extends ApiController } if ($deletable instanceof Book || $deletable instanceof Chapter) { - $countsToLoad = ['pages' => $withTrashedQuery]; + $countsToLoad = ['pages' => static::withTrashedQuery(...)]; if ($deletable instanceof Book) { - $countsToLoad['chapters'] = $withTrashedQuery; + $countsToLoad['chapters'] = static::withTrashedQuery(...); } $deletable->loadCount($countsToLoad); } } + + /** + * @param HasMany $query + */ + protected static function withTrashedQuery(HasMany $query): void + { + $query->withTrashed(); + } } diff --git a/app/Entities/Models/Book.php b/app/Entities/Models/Book.php index 88e3b85ba..5f54e0f6a 100644 --- a/app/Entities/Models/Book.php +++ b/app/Entities/Models/Book.php @@ -95,6 +95,7 @@ class Book extends Entity implements CoverImageInterface, HtmlDescriptionInterfa /** * Get all pages within this book. + * @return HasMany */ public function pages(): HasMany { @@ -111,7 +112,7 @@ class Book extends Entity implements CoverImageInterface, HtmlDescriptionInterfa /** * Get all chapters within this book. - * @return HasMany + * @return HasMany */ public function chapters(): HasMany { diff --git a/app/Entities/Models/Bookshelf.php b/app/Entities/Models/Bookshelf.php index 5b403d9c0..9ae52abcb 100644 --- a/app/Entities/Models/Bookshelf.php +++ b/app/Entities/Models/Bookshelf.php @@ -70,6 +70,7 @@ class Bookshelf extends Entity implements CoverImageInterface, HtmlDescriptionIn /** * Get the cover image of the shelf. + * @return BelongsTo */ public function cover(): BelongsTo { diff --git a/app/Entities/Models/Chapter.php b/app/Entities/Models/Chapter.php index 03e819c06..d70a49e7a 100644 --- a/app/Entities/Models/Chapter.php +++ b/app/Entities/Models/Chapter.php @@ -27,7 +27,7 @@ class Chapter extends BookChild implements HtmlDescriptionInterface /** * Get the pages that this chapter contains. * - * @return HasMany + * @return HasMany */ public function pages(string $dir = 'ASC'): HasMany { @@ -60,7 +60,7 @@ class Chapter extends BookChild implements HtmlDescriptionInterface /** * Get the visible pages in this chapter. - * @returns Collection + * @return Collection */ public function getVisiblePages(): Collection { diff --git a/app/Entities/Models/Entity.php b/app/Entities/Models/Entity.php index 46b29f93b..31511aa83 100644 --- a/app/Entities/Models/Entity.php +++ b/app/Entities/Models/Entity.php @@ -12,7 +12,7 @@ use BookStack\Activity\Models\View; use BookStack\Activity\Models\Viewable; use BookStack\Activity\Models\Watch; use BookStack\App\Model; -use BookStack\App\Sluggable; +use BookStack\App\SluggableInterface; use BookStack\Entities\Tools\SlugGenerator; use BookStack\Permissions\JointPermissionBuilder; use BookStack\Permissions\Models\EntityPermission; @@ -22,7 +22,8 @@ use BookStack\References\Reference; use BookStack\Search\SearchIndex; use BookStack\Search\SearchTerm; use BookStack\Users\Models\HasCreatorAndUpdater; -use BookStack\Users\Models\HasOwner; +use BookStack\Users\Models\OwnableInterface; +use BookStack\Users\Models\User; use Carbon\Carbon; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Collection; @@ -43,17 +44,23 @@ use Illuminate\Database\Eloquent\SoftDeletes; * @property Carbon $deleted_at * @property int $created_by * @property int $updated_by + * @property int $owned_by * @property Collection $tags * * @method static Entity|Builder visible() * @method static Builder withLastView() * @method static Builder withViewCount() */ -abstract class Entity extends Model implements Sluggable, Favouritable, Viewable, DeletableInterface, Loggable +abstract class Entity extends Model implements + SluggableInterface, + Favouritable, + Viewable, + DeletableInterface, + OwnableInterface, + Loggable { use SoftDeletes; use HasCreatorAndUpdater; - use HasOwner; /** * @var string - Name of property where the main text content is found @@ -200,6 +207,20 @@ abstract class Entity extends Model implements Sluggable, Favouritable, Viewable return $this->morphMany(JointPermission::class, 'entity'); } + /** + * Get the user who owns this entity. + * @return BelongsTo + */ + public function ownedBy(): BelongsTo + { + return $this->belongsTo(User::class, 'owned_by'); + } + + public function getOwnerFieldName(): string + { + return 'owned_by'; + } + /** * Get the related delete records for this entity. */ @@ -318,7 +339,7 @@ abstract class Entity extends Model implements Sluggable, Favouritable, Viewable */ public function refreshSlug(): string { - $this->slug = app()->make(SlugGenerator::class)->generate($this); + $this->slug = app()->make(SlugGenerator::class)->generate($this, $this->name); return $this->slug; } diff --git a/app/Entities/Queries/BookQueries.php b/app/Entities/Queries/BookQueries.php index 534640621..5ff22252c 100644 --- a/app/Entities/Queries/BookQueries.php +++ b/app/Entities/Queries/BookQueries.php @@ -13,6 +13,9 @@ class BookQueries implements ProvidesEntityQueries 'created_at', 'updated_at', 'image_id', 'owned_by', ]; + /** + * @return Builder + */ public function start(): Builder { return Book::query(); diff --git a/app/Entities/Queries/BookshelfQueries.php b/app/Entities/Queries/BookshelfQueries.php index 19717fb7c..874bc7f2f 100644 --- a/app/Entities/Queries/BookshelfQueries.php +++ b/app/Entities/Queries/BookshelfQueries.php @@ -13,6 +13,9 @@ class BookshelfQueries implements ProvidesEntityQueries 'created_at', 'updated_at', 'image_id', 'owned_by', ]; + /** + * @return Builder + */ public function start(): Builder { return Bookshelf::query(); diff --git a/app/Entities/Queries/EntityQueries.php b/app/Entities/Queries/EntityQueries.php index 36dc6c0bc..0d2cd7acf 100644 --- a/app/Entities/Queries/EntityQueries.php +++ b/app/Entities/Queries/EntityQueries.php @@ -35,6 +35,7 @@ class EntityQueries /** * Start a query of visible entities of the given type, * suitable for listing display. + * @return Builder */ public function visibleForList(string $entityType): Builder { @@ -44,7 +45,6 @@ class EntityQueries protected function getQueriesForType(string $type): ProvidesEntityQueries { - /** @var ?ProvidesEntityQueries $queries */ $queries = match ($type) { 'page' => $this->pages, 'chapter' => $this->chapters, diff --git a/app/Entities/Queries/ProvidesEntityQueries.php b/app/Entities/Queries/ProvidesEntityQueries.php index 611d0ae52..1f8a71ae2 100644 --- a/app/Entities/Queries/ProvidesEntityQueries.php +++ b/app/Entities/Queries/ProvidesEntityQueries.php @@ -22,13 +22,14 @@ interface ProvidesEntityQueries public function start(): Builder; /** - * Find the entity of the given ID, or return null if not found. + * Find the entity of the given ID or return null if not found. */ public function findVisibleById(int $id): ?Entity; /** * Start a query for items that are visible, with selection * configured for list display of this item. + * @return Builder */ public function visibleForList(): Builder; } diff --git a/app/Entities/Repos/BaseRepo.php b/app/Entities/Repos/BaseRepo.php index f4d05d8af..bfc01a58d 100644 --- a/app/Entities/Repos/BaseRepo.php +++ b/app/Entities/Repos/BaseRepo.php @@ -89,12 +89,10 @@ class BaseRepo /** * Update the given items' cover image, or clear it. * - * @param Entity&CoverImageInterface $entity - * * @throws ImageUploadException * @throws \Exception */ - public function updateCoverImage($entity, ?UploadedFile $coverImage, bool $removeImage = false) + public function updateCoverImage(Entity&CoverImageInterface $entity, ?UploadedFile $coverImage, bool $removeImage = false) { if ($coverImage) { $imageType = $entity->coverImageTypeKey(); @@ -106,7 +104,7 @@ class BaseRepo if ($removeImage) { $this->imageRepo->destroyImage($entity->cover()->first()); - $entity->image_id = 0; + $entity->cover()->dissociate(); $entity->save(); } } diff --git a/app/Entities/Tools/SlugGenerator.php b/app/Entities/Tools/SlugGenerator.php index 5df300bb0..fb9123187 100644 --- a/app/Entities/Tools/SlugGenerator.php +++ b/app/Entities/Tools/SlugGenerator.php @@ -3,7 +3,7 @@ namespace BookStack\Entities\Tools; use BookStack\App\Model; -use BookStack\App\Sluggable; +use BookStack\App\SluggableInterface; use BookStack\Entities\Models\BookChild; use Illuminate\Support\Str; @@ -13,9 +13,9 @@ class SlugGenerator * Generate a fresh slug for the given entity. * The slug will be generated so that it doesn't conflict within the same parent item. */ - public function generate(Sluggable $model): string + public function generate(SluggableInterface&Model $model, string $slugSource): string { - $slug = $this->formatNameAsSlug($model->name); + $slug = $this->formatNameAsSlug($slugSource); while ($this->slugInUse($slug, $model)) { $slug .= '-' . Str::random(3); } @@ -24,7 +24,7 @@ class SlugGenerator } /** - * Format a name as a url slug. + * Format a name as a URL slug. */ protected function formatNameAsSlug(string $name): string { @@ -39,10 +39,8 @@ class SlugGenerator /** * Check if a slug is already in-use for this * type of model within the same parent. - * - * @param Sluggable&Model $model */ - protected function slugInUse(string $slug, Sluggable $model): bool + protected function slugInUse(string $slug, SluggableInterface&Model $model): bool { $query = $model->newQuery()->where('slug', '=', $slug); diff --git a/app/Permissions/PermissionApplicator.php b/app/Permissions/PermissionApplicator.php index ce4a543fd..e59b8ab67 100644 --- a/app/Permissions/PermissionApplicator.php +++ b/app/Permissions/PermissionApplicator.php @@ -7,8 +7,7 @@ use BookStack\Entities\EntityProvider; use BookStack\Entities\Models\Entity; use BookStack\Entities\Models\Page; use BookStack\Permissions\Models\EntityPermission; -use BookStack\Users\Models\HasCreatorAndUpdater; -use BookStack\Users\Models\HasOwner; +use BookStack\Users\Models\OwnableInterface; use BookStack\Users\Models\User; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Query\Builder as QueryBuilder; @@ -24,10 +23,8 @@ class PermissionApplicator /** * Checks if an entity has a restriction set upon it. - * - * @param Model&(HasCreatorAndUpdater|HasOwner) $ownable */ - public function checkOwnableUserAccess(Model $ownable, string $permission): bool + public function checkOwnableUserAccess(Model&OwnableInterface $ownable, string $permission): bool { $explodedPermission = explode('-', $permission); $action = $explodedPermission[1] ?? $explodedPermission[0]; @@ -39,7 +36,7 @@ class PermissionApplicator $allRolePermission = $user->can($fullPermission . '-all'); $ownRolePermission = $user->can($fullPermission . '-own'); $nonJointPermissions = ['restrictions', 'image', 'attachment', 'comment']; - $ownerField = ($ownable instanceof Entity) ? 'owned_by' : 'created_by'; + $ownerField = $ownable->getOwnerFieldName(); $ownableFieldVal = $ownable->getAttribute($ownerField); if (is_null($ownableFieldVal)) { @@ -49,11 +46,15 @@ class PermissionApplicator $isOwner = $user->id === $ownableFieldVal; $hasRolePermission = $allRolePermission || ($isOwner && $ownRolePermission); - // Handle non entity specific jointPermissions + // Handle non-entity-specific jointPermissions if (in_array($explodedPermission[0], $nonJointPermissions)) { return $hasRolePermission; } + if (!($ownable instanceof Entity)) { + return false; + } + $hasApplicableEntityPermissions = $this->hasEntityPermission($ownable, $userRoleIds, $action); return is_null($hasApplicableEntityPermissions) ? $hasRolePermission : $hasApplicableEntityPermissions; diff --git a/app/References/CrossLinkParser.php b/app/References/CrossLinkParser.php index 1fd4c1b3e..3fb00be84 100644 --- a/app/References/CrossLinkParser.php +++ b/app/References/CrossLinkParser.php @@ -48,7 +48,7 @@ class CrossLinkParser /** * Get a list of href values from the given document. * - * @returns string[] + * @return string[] */ protected function getLinksFromContent(string $html): array { diff --git a/app/Search/SearchIndex.php b/app/Search/SearchIndex.php index 844e3584b..117d069ea 100644 --- a/app/Search/SearchIndex.php +++ b/app/Search/SearchIndex.php @@ -119,7 +119,7 @@ class SearchIndex * Create a scored term array from the given text, where the keys are the terms * and the values are their scores. * - * @returns array + * @return array */ protected function generateTermScoreMapFromText(string $text, float $scoreAdjustment = 1): array { @@ -136,7 +136,7 @@ class SearchIndex * Create a scored term array from the given HTML, where the keys are the terms * and the values are their scores. * - * @returns array + * @return array */ protected function generateTermScoreMapFromHtml(string $html): array { @@ -177,7 +177,7 @@ class SearchIndex * * @param Tag[] $tags * - * @returns array + * @return array */ protected function generateTermScoreMapFromTags(array $tags): array { @@ -199,7 +199,7 @@ class SearchIndex * For the given text, return an array where the keys are the unique term words * and the values are the frequency of that term. * - * @returns array + * @return array */ protected function textToTermCountMap(string $text): array { @@ -243,7 +243,7 @@ class SearchIndex * For the given entity, Generate an array of term data details. * Is the raw term data, not instances of SearchTerm models. * - * @returns array{term: string, score: float, entity_id: int, entity_type: string}[] + * @return array{term: string, score: float, entity_id: int, entity_type: string}[] */ protected function entityToTermDataArray(Entity $entity): array { @@ -279,7 +279,7 @@ class SearchIndex * * @param array[] ...$scoreMaps * - * @returns array + * @return array */ protected function mergeTermScoreMaps(...$scoreMaps): array { diff --git a/app/Sorting/BookSorter.php b/app/Sorting/BookSorter.php index cf41a6a94..e627d66fd 100644 --- a/app/Sorting/BookSorter.php +++ b/app/Sorting/BookSorter.php @@ -78,7 +78,7 @@ class BookSorter * Sort the books content using the given sort map. * Returns a list of books that were involved in the operation. * - * @returns Book[] + * @return Book[] */ public function sortUsingMap(BookSortMap $sortMap): array { diff --git a/app/Theming/ThemeEvents.php b/app/Theming/ThemeEvents.php index 2d4900c96..44630acae 100644 --- a/app/Theming/ThemeEvents.php +++ b/app/Theming/ThemeEvents.php @@ -62,7 +62,7 @@ class ThemeEvents * * @param string $authSystem * @param array $userData - * @returns bool|null + * @return bool|null */ const AUTH_PRE_REGISTER = 'auth_pre_register'; @@ -83,7 +83,7 @@ class ThemeEvents * If the listener returns a non-null value, that will be used as an environment instead. * * @param \League\CommonMark\Environment\Environment $environment - * @returns \League\CommonMark\Environment\Environment|null + * @return \League\CommonMark\Environment\Environment|null */ const COMMONMARK_ENVIRONMENT_CONFIGURE = 'commonmark_environment_configure'; @@ -96,7 +96,7 @@ class ThemeEvents * * @param array $idTokenData * @param array $accessTokenData - * @returns array|null + * @return array|null */ const OIDC_ID_TOKEN_PRE_VALIDATE = 'oidc_id_token_pre_validate'; @@ -142,7 +142,7 @@ class ThemeEvents * Return values, if provided, will be used as a new response to use. * * @param \Illuminate\Http\Request $request - * @returns \Illuminate\Http\Response|null + * @return \Illuminate\Http\Response|null */ const WEB_MIDDLEWARE_BEFORE = 'web_middleware_before'; @@ -154,7 +154,7 @@ class ThemeEvents * * @param \Illuminate\Http\Request $request * @param \Illuminate\Http\Response|\Symfony\Component\HttpFoundation\BinaryFileResponse $response - * @returns \Illuminate\Http\Response|null + * @return \Illuminate\Http\Response|null */ const WEB_MIDDLEWARE_AFTER = 'web_middleware_after'; @@ -173,7 +173,7 @@ class ThemeEvents * @param string|\BookStack\Activity\Models\Loggable $detail * @param \BookStack\Users\Models\User $initiator * @param int $initiatedTime - * @returns array|null + * @return array|null */ const WEBHOOK_CALL_BEFORE = 'webhook_call_before'; } diff --git a/app/Uploads/Attachment.php b/app/Uploads/Attachment.php index 57d7cb334..05227243a 100644 --- a/app/Uploads/Attachment.php +++ b/app/Uploads/Attachment.php @@ -8,6 +8,7 @@ use BookStack\Entities\Models\Page; use BookStack\Permissions\Models\JointPermission; use BookStack\Permissions\PermissionApplicator; use BookStack\Users\Models\HasCreatorAndUpdater; +use BookStack\Users\Models\OwnableInterface; use BookStack\Users\Models\User; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Factories\HasFactory; @@ -27,7 +28,7 @@ use Illuminate\Database\Eloquent\Relations\HasMany; * * @method static Entity|Builder visible() */ -class Attachment extends Model +class Attachment extends Model implements OwnableInterface { use HasCreatorAndUpdater; use HasFactory; diff --git a/app/Uploads/Image.php b/app/Uploads/Image.php index 0a267a644..ec8f8be0f 100644 --- a/app/Uploads/Image.php +++ b/app/Uploads/Image.php @@ -7,6 +7,7 @@ use BookStack\Entities\Models\Page; use BookStack\Permissions\Models\JointPermission; use BookStack\Permissions\PermissionApplicator; use BookStack\Users\Models\HasCreatorAndUpdater; +use BookStack\Users\Models\OwnableInterface; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Relations\HasMany; @@ -21,7 +22,7 @@ use Illuminate\Database\Eloquent\Relations\HasMany; * @property int $created_by * @property int $updated_by */ -class Image extends Model +class Image extends Model implements OwnableInterface { use HasFactory; use HasCreatorAndUpdater; diff --git a/app/Uploads/ImageService.php b/app/Uploads/ImageService.php index a8f144517..458f0102d 100644 --- a/app/Uploads/ImageService.php +++ b/app/Uploads/ImageService.php @@ -138,7 +138,7 @@ class ImageService * Get the raw data content from an image. * * @throws Exception - * @returns ?resource + * @return ?resource */ public function getImageStream(Image $image): mixed { diff --git a/app/Uploads/ImageStorageDisk.php b/app/Uploads/ImageStorageDisk.php index f2667d993..36d6721de 100644 --- a/app/Uploads/ImageStorageDisk.php +++ b/app/Uploads/ImageStorageDisk.php @@ -61,7 +61,7 @@ class ImageStorageDisk /** * Get a stream to the file at the given path. - * @returns ?resource + * @return ?resource */ public function stream(string $path): mixed { diff --git a/app/Users/Controllers/UserApiController.php b/app/Users/Controllers/UserApiController.php index bb2570b31..351893b03 100644 --- a/app/Users/Controllers/UserApiController.php +++ b/app/Users/Controllers/UserApiController.php @@ -81,7 +81,7 @@ class UserApiController extends ApiController return $this->apiListingResponse($users, [ 'id', 'name', 'slug', 'email', 'external_auth_id', 'created_at', 'updated_at', 'last_activity_at', - ], [Closure::fromCallable([$this, 'listFormatter'])]); + ], [$this->listFormatter(...)]); } /** diff --git a/app/Users/Models/HasCreatorAndUpdater.php b/app/Users/Models/HasCreatorAndUpdater.php index 9fb24ead9..bb05de11a 100644 --- a/app/Users/Models/HasCreatorAndUpdater.php +++ b/app/Users/Models/HasCreatorAndUpdater.php @@ -27,4 +27,9 @@ trait HasCreatorAndUpdater { return $this->belongsTo(User::class, 'updated_by'); } + + public function getOwnerFieldName(): string + { + return 'created_by'; + } } diff --git a/app/Users/Models/HasOwner.php b/app/Users/Models/HasOwner.php deleted file mode 100644 index eb830ef6f..000000000 --- a/app/Users/Models/HasOwner.php +++ /dev/null @@ -1,19 +0,0 @@ -belongsTo(User::class, 'owned_by'); - } -} diff --git a/app/Users/Models/OwnableInterface.php b/app/Users/Models/OwnableInterface.php new file mode 100644 index 000000000..8f738487f --- /dev/null +++ b/app/Users/Models/OwnableInterface.php @@ -0,0 +1,8 @@ + */ public function permissions(): BelongsToMany { diff --git a/app/Users/Models/User.php b/app/Users/Models/User.php index 0d437418b..b6989ce2d 100644 --- a/app/Users/Models/User.php +++ b/app/Users/Models/User.php @@ -10,7 +10,7 @@ use BookStack\Activity\Models\Loggable; use BookStack\Activity\Models\Watch; use BookStack\Api\ApiToken; use BookStack\App\Model; -use BookStack\App\Sluggable; +use BookStack\App\SluggableInterface; use BookStack\Entities\Tools\SlugGenerator; use BookStack\Translation\LocaleDefinition; use BookStack\Translation\LocaleManager; @@ -47,7 +47,7 @@ use Illuminate\Support\Collection; * @property Collection $mfaValues * @property ?Image $avatar */ -class User extends Model implements AuthenticatableContract, CanResetPasswordContract, Loggable, Sluggable +class User extends Model implements AuthenticatableContract, CanResetPasswordContract, Loggable, SluggableInterface { use HasFactory; use Authenticatable; @@ -372,7 +372,7 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon */ public function refreshSlug(): string { - $this->slug = app()->make(SlugGenerator::class)->generate($this); + $this->slug = app()->make(SlugGenerator::class)->generate($this, $this->name); return $this->slug; } diff --git a/phpcs.xml b/phpcs.xml index 8d4c6b702..8337d5aac 100644 --- a/phpcs.xml +++ b/phpcs.xml @@ -14,7 +14,7 @@ - + ./tests/* diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 0f2021383..9dfd9d29e 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -7,7 +7,7 @@ parameters: - app # The level 8 is the highest level - level: 1 + level: 2 phpVersion: min: 80200 From e05ec7da361027126e538bbe5908d0bfdf1ede4a Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Wed, 3 Sep 2025 10:47:45 +0100 Subject: [PATCH 3/5] Maintenance: Addressed a range of phpstan level 3 issues --- app/Access/ExternalBaseUserProvider.php | 24 +--- .../Guards/ExternalBaseSessionGuard.php | 111 ++++-------------- app/Activity/Models/Comment.php | 2 +- app/App/Providers/AuthServiceProvider.php | 4 +- app/App/Providers/EventServiceProvider.php | 2 +- .../Controllers/PageRevisionController.php | 2 +- app/Entities/Models/PageRevision.php | 2 +- app/Exceptions/Handler.php | 4 +- app/Exports/ImportRepo.php | 3 + .../ZipExports/Models/ZipExportAttachment.php | 6 +- .../ZipExports/Models/ZipExportBook.php | 6 +- .../ZipExports/Models/ZipExportChapter.php | 6 +- .../ZipExports/Models/ZipExportImage.php | 6 +- .../ZipExports/Models/ZipExportModel.php | 4 +- .../ZipExports/Models/ZipExportPage.php | 6 +- .../ZipExports/Models/ZipExportTag.php | 6 +- app/Http/Kernel.php | 6 +- app/Http/Middleware/EncryptCookies.php | 2 +- .../PreventRequestsDuringMaintenance.php | 2 +- app/Http/Middleware/TrimStrings.php | 2 +- app/Http/Middleware/TrustProxies.php | 2 +- app/Http/Middleware/VerifyCsrfToken.php | 2 +- app/Search/SearchOptionSet.php | 3 + app/Search/SearchRunner.php | 2 +- app/Users/Models/User.php | 13 +- app/Util/OutOfMemoryHandler.php | 7 +- 26 files changed, 84 insertions(+), 151 deletions(-) diff --git a/app/Access/ExternalBaseUserProvider.php b/app/Access/ExternalBaseUserProvider.php index 2b5ddfbf3..2165fd459 100644 --- a/app/Access/ExternalBaseUserProvider.php +++ b/app/Access/ExternalBaseUserProvider.php @@ -2,33 +2,18 @@ namespace BookStack\Access; +use BookStack\Users\Models\User; use Illuminate\Contracts\Auth\Authenticatable; use Illuminate\Contracts\Auth\UserProvider; -use Illuminate\Database\Eloquent\Model; class ExternalBaseUserProvider implements UserProvider { - public function __construct( - protected string $model - ) { - } - - /** - * Create a new instance of the model. - */ - public function createModel(): Model - { - $class = '\\' . ltrim($this->model, '\\'); - - return new $class(); - } - /** * Retrieve a user by their unique identifier. */ public function retrieveById(mixed $identifier): ?Authenticatable { - return $this->createModel()->newQuery()->find($identifier); + return User::query()->find($identifier); } /** @@ -59,10 +44,7 @@ class ExternalBaseUserProvider implements UserProvider */ public function retrieveByCredentials(array $credentials): ?Authenticatable { - // Search current user base by looking up a uid - $model = $this->createModel(); - - return $model->newQuery() + return User::query() ->where('external_auth_id', $credentials['external_auth_id']) ->first(); } diff --git a/app/Access/Guards/ExternalBaseSessionGuard.php b/app/Access/Guards/ExternalBaseSessionGuard.php index 2d15422a5..899e93c36 100644 --- a/app/Access/Guards/ExternalBaseSessionGuard.php +++ b/app/Access/Guards/ExternalBaseSessionGuard.php @@ -4,7 +4,7 @@ namespace BookStack\Access\Guards; use BookStack\Access\RegistrationService; use Illuminate\Auth\GuardHelpers; -use Illuminate\Contracts\Auth\Authenticatable as AuthenticatableContract; +use Illuminate\Contracts\Auth\Authenticatable; use Illuminate\Contracts\Auth\StatefulGuard; use Illuminate\Contracts\Auth\UserProvider; use Illuminate\Contracts\Session\Session; @@ -24,43 +24,31 @@ class ExternalBaseSessionGuard implements StatefulGuard * The name of the Guard. Typically "session". * * Corresponds to guard name in authentication configuration. - * - * @var string */ - protected $name; + protected readonly string $name; /** * The user we last attempted to retrieve. - * - * @var \Illuminate\Contracts\Auth\Authenticatable */ - protected $lastAttempted; + protected Authenticatable $lastAttempted; /** * The session used by the guard. - * - * @var \Illuminate\Contracts\Session\Session */ - protected $session; + protected Session $session; /** * Indicates if the logout method has been called. - * - * @var bool */ - protected $loggedOut = false; + protected bool $loggedOut = false; /** * Service to handle common registration actions. - * - * @var RegistrationService */ - protected $registrationService; + protected RegistrationService $registrationService; /** * Create a new authentication guard. - * - * @return void */ public function __construct(string $name, UserProvider $provider, Session $session, RegistrationService $registrationService) { @@ -72,13 +60,11 @@ class ExternalBaseSessionGuard implements StatefulGuard /** * Get the currently authenticated user. - * - * @return \Illuminate\Contracts\Auth\Authenticatable|null */ - public function user() + public function user(): Authenticatable|null { if ($this->loggedOut) { - return; + return null; } // If we've already retrieved the user for the current request we can just @@ -101,13 +87,11 @@ class ExternalBaseSessionGuard implements StatefulGuard /** * Get the ID for the currently authenticated user. - * - * @return int|null */ - public function id() + public function id(): int|null { if ($this->loggedOut) { - return; + return null; } return $this->user() @@ -117,12 +101,8 @@ class ExternalBaseSessionGuard implements StatefulGuard /** * Log a user into the application without sessions or cookies. - * - * @param array $credentials - * - * @return bool */ - public function once(array $credentials = []) + public function once(array $credentials = []): bool { if ($this->validate($credentials)) { $this->setUser($this->lastAttempted); @@ -135,12 +115,8 @@ class ExternalBaseSessionGuard implements StatefulGuard /** * Log the given user ID into the application without sessions or cookies. - * - * @param mixed $id - * - * @return \Illuminate\Contracts\Auth\Authenticatable|false */ - public function onceUsingId($id) + public function onceUsingId($id): Authenticatable|false { if (!is_null($user = $this->provider->retrieveById($id))) { $this->setUser($user); @@ -153,38 +129,26 @@ class ExternalBaseSessionGuard implements StatefulGuard /** * Validate a user's credentials. - * - * @param array $credentials - * - * @return bool */ - public function validate(array $credentials = []) + public function validate(array $credentials = []): bool { return false; } /** * Attempt to authenticate a user using the given credentials. - * - * @param array $credentials - * @param bool $remember - * - * @return bool + * @param bool $remember */ - public function attempt(array $credentials = [], $remember = false) + public function attempt(array $credentials = [], $remember = false): bool { return false; } /** * Log the given user ID into the application. - * - * @param mixed $id * @param bool $remember - * - * @return \Illuminate\Contracts\Auth\Authenticatable|false */ - public function loginUsingId($id, $remember = false) + public function loginUsingId(mixed $id, $remember = false): Authenticatable|false { // Always return false as to disable this method, // Logins should route through LoginService. @@ -194,12 +158,9 @@ class ExternalBaseSessionGuard implements StatefulGuard /** * Log a user into the application. * - * @param \Illuminate\Contracts\Auth\Authenticatable $user - * @param bool $remember - * - * @return void + * @param bool $remember */ - public function login(AuthenticatableContract $user, $remember = false) + public function login(Authenticatable $user, $remember = false): void { $this->updateSession($user->getAuthIdentifier()); @@ -208,12 +169,8 @@ class ExternalBaseSessionGuard implements StatefulGuard /** * Update the session with the given ID. - * - * @param string $id - * - * @return void */ - protected function updateSession($id) + protected function updateSession(string|int $id): void { $this->session->put($this->getName(), $id); @@ -222,10 +179,8 @@ class ExternalBaseSessionGuard implements StatefulGuard /** * Log the user out of the application. - * - * @return void */ - public function logout() + public function logout(): void { $this->clearUserDataFromStorage(); @@ -239,62 +194,48 @@ class ExternalBaseSessionGuard implements StatefulGuard /** * Remove the user data from the session and cookies. - * - * @return void */ - protected function clearUserDataFromStorage() + protected function clearUserDataFromStorage(): void { $this->session->remove($this->getName()); } /** * Get the last user we attempted to authenticate. - * - * @return \Illuminate\Contracts\Auth\Authenticatable */ - public function getLastAttempted() + public function getLastAttempted(): Authenticatable { return $this->lastAttempted; } /** * Get a unique identifier for the auth session value. - * - * @return string */ - public function getName() + public function getName(): string { return 'login_' . $this->name . '_' . sha1(static::class); } /** * Determine if the user was authenticated via "remember me" cookie. - * - * @return bool */ - public function viaRemember() + public function viaRemember(): bool { return false; } /** * Return the currently cached user. - * - * @return \Illuminate\Contracts\Auth\Authenticatable|null */ - public function getUser() + public function getUser(): Authenticatable|null { return $this->user; } /** * Set the current user. - * - * @param \Illuminate\Contracts\Auth\Authenticatable $user - * - * @return $this */ - public function setUser(AuthenticatableContract $user) + public function setUser(Authenticatable $user): self { $this->user = $user; diff --git a/app/Activity/Models/Comment.php b/app/Activity/Models/Comment.php index 6b05a9bab..0cb83d61e 100644 --- a/app/Activity/Models/Comment.php +++ b/app/Activity/Models/Comment.php @@ -39,7 +39,7 @@ class Comment extends Model implements Loggable, OwnableInterface /** * Get the parent comment this is in reply to (if existing). - * @return BelongsTo + * @return BelongsTo */ public function parent(): BelongsTo { diff --git a/app/App/Providers/AuthServiceProvider.php b/app/App/Providers/AuthServiceProvider.php index 23c339079..6a8162521 100644 --- a/app/App/Providers/AuthServiceProvider.php +++ b/app/App/Providers/AuthServiceProvider.php @@ -59,8 +59,8 @@ class AuthServiceProvider extends ServiceProvider */ public function register(): void { - Auth::provider('external-users', function ($app, array $config) { - return new ExternalBaseUserProvider($config['model']); + Auth::provider('external-users', function () { + return new ExternalBaseUserProvider(); }); // Bind and provide the default system user as a singleton to the app instance when needed. diff --git a/app/App/Providers/EventServiceProvider.php b/app/App/Providers/EventServiceProvider.php index 34ab7cfef..60e78efe0 100644 --- a/app/App/Providers/EventServiceProvider.php +++ b/app/App/Providers/EventServiceProvider.php @@ -15,7 +15,7 @@ class EventServiceProvider extends ServiceProvider /** * The event listener mappings for the application. * - * @var array> + * @var array> */ protected $listen = [ SocialiteWasCalled::class => [ diff --git a/app/Entities/Controllers/PageRevisionController.php b/app/Entities/Controllers/PageRevisionController.php index 4985c39f3..c43eea08b 100644 --- a/app/Entities/Controllers/PageRevisionController.php +++ b/app/Entities/Controllers/PageRevisionController.php @@ -98,7 +98,7 @@ class PageRevisionController extends Controller throw new NotFoundException(); } - $prev = $revision->getPrevious(); + $prev = $revision->getPreviousRevision(); $prevContent = $prev->html ?? ''; $diff = Diff::excecute($prevContent, $revision->html); diff --git a/app/Entities/Models/PageRevision.php b/app/Entities/Models/PageRevision.php index 10ff6d901..1a6c980e1 100644 --- a/app/Entities/Models/PageRevision.php +++ b/app/Entities/Models/PageRevision.php @@ -60,7 +60,7 @@ class PageRevision extends Model implements Loggable /** * Get the previous revision for the same page if existing. */ - public function getPrevious(): ?PageRevision + public function getPreviousRevision(): ?PageRevision { $id = static::newQuery()->where('page_id', '=', $this->page_id) ->where('id', '<', $this->id) diff --git a/app/Exceptions/Handler.php b/app/Exceptions/Handler.php index 61e126327..b8c3fa192 100644 --- a/app/Exceptions/Handler.php +++ b/app/Exceptions/Handler.php @@ -94,7 +94,7 @@ class Handler extends ExceptionHandler * If the callable returns a response, this response will be returned * to the request upon error. */ - public function prepareForOutOfMemory(callable $onOutOfMemory) + public function prepareForOutOfMemory(callable $onOutOfMemory): void { $this->onOutOfMemory = $onOutOfMemory; } @@ -102,7 +102,7 @@ class Handler extends ExceptionHandler /** * Forget the current out of memory handler, if existing. */ - public function forgetOutOfMemoryHandler() + public function forgetOutOfMemoryHandler(): void { $this->onOutOfMemory = null; } diff --git a/app/Exports/ImportRepo.php b/app/Exports/ImportRepo.php index e030a88d2..896af903a 100644 --- a/app/Exports/ImportRepo.php +++ b/app/Exports/ImportRepo.php @@ -39,6 +39,9 @@ class ImportRepo return $this->queryVisible()->get(); } + /** + * @return Builder + */ public function queryVisible(): Builder { $query = Import::query(); diff --git a/app/Exports/ZipExports/Models/ZipExportAttachment.php b/app/Exports/ZipExports/Models/ZipExportAttachment.php index 4f5b2f236..97995738f 100644 --- a/app/Exports/ZipExports/Models/ZipExportAttachment.php +++ b/app/Exports/ZipExports/Models/ZipExportAttachment.php @@ -6,7 +6,7 @@ use BookStack\Exports\ZipExports\ZipExportFiles; use BookStack\Exports\ZipExports\ZipValidationHelper; use BookStack\Uploads\Attachment; -class ZipExportAttachment extends ZipExportModel +final class ZipExportAttachment extends ZipExportModel { public ?int $id = null; public string $name; @@ -52,9 +52,9 @@ class ZipExportAttachment extends ZipExportModel return $context->validateData($data, $rules); } - public static function fromArray(array $data): self + public static function fromArray(array $data): static { - $model = new self(); + $model = new static(); $model->id = $data['id'] ?? null; $model->name = $data['name']; diff --git a/app/Exports/ZipExports/Models/ZipExportBook.php b/app/Exports/ZipExports/Models/ZipExportBook.php index 39176ded4..6c51ea337 100644 --- a/app/Exports/ZipExports/Models/ZipExportBook.php +++ b/app/Exports/ZipExports/Models/ZipExportBook.php @@ -8,7 +8,7 @@ use BookStack\Entities\Models\Page; use BookStack\Exports\ZipExports\ZipExportFiles; use BookStack\Exports\ZipExports\ZipValidationHelper; -class ZipExportBook extends ZipExportModel +final class ZipExportBook extends ZipExportModel { public ?int $id = null; public string $name; @@ -101,9 +101,9 @@ class ZipExportBook extends ZipExportModel return $errors; } - public static function fromArray(array $data): self + public static function fromArray(array $data): static { - $model = new self(); + $model = new static(); $model->id = $data['id'] ?? null; $model->name = $data['name']; diff --git a/app/Exports/ZipExports/Models/ZipExportChapter.php b/app/Exports/ZipExports/Models/ZipExportChapter.php index bf2dc78f8..260191a3e 100644 --- a/app/Exports/ZipExports/Models/ZipExportChapter.php +++ b/app/Exports/ZipExports/Models/ZipExportChapter.php @@ -7,7 +7,7 @@ use BookStack\Entities\Models\Page; use BookStack\Exports\ZipExports\ZipExportFiles; use BookStack\Exports\ZipExports\ZipValidationHelper; -class ZipExportChapter extends ZipExportModel +final class ZipExportChapter extends ZipExportModel { public ?int $id = null; public string $name; @@ -79,9 +79,9 @@ class ZipExportChapter extends ZipExportModel return $errors; } - public static function fromArray(array $data): self + public static function fromArray(array $data): static { - $model = new self(); + $model = new static(); $model->id = $data['id'] ?? null; $model->name = $data['name']; diff --git a/app/Exports/ZipExports/Models/ZipExportImage.php b/app/Exports/ZipExports/Models/ZipExportImage.php index e0e7d1198..4c71af0c3 100644 --- a/app/Exports/ZipExports/Models/ZipExportImage.php +++ b/app/Exports/ZipExports/Models/ZipExportImage.php @@ -7,7 +7,7 @@ use BookStack\Exports\ZipExports\ZipValidationHelper; use BookStack\Uploads\Image; use Illuminate\Validation\Rule; -class ZipExportImage extends ZipExportModel +final class ZipExportImage extends ZipExportModel { public ?int $id = null; public string $name; @@ -43,9 +43,9 @@ class ZipExportImage extends ZipExportModel return $context->validateData($data, $rules); } - public static function fromArray(array $data): self + public static function fromArray(array $data): static { - $model = new self(); + $model = new static(); $model->id = $data['id'] ?? null; $model->name = $data['name']; diff --git a/app/Exports/ZipExports/Models/ZipExportModel.php b/app/Exports/ZipExports/Models/ZipExportModel.php index d3a8c3567..38001c628 100644 --- a/app/Exports/ZipExports/Models/ZipExportModel.php +++ b/app/Exports/ZipExports/Models/ZipExportModel.php @@ -30,12 +30,12 @@ abstract class ZipExportModel implements JsonSerializable /** * Decode the array of data into this export model. */ - abstract public static function fromArray(array $data): self; + abstract public static function fromArray(array $data): static; /** * Decode an array of array data into an array of export models. * @param array[] $data - * @return self[] + * @return static[] */ public static function fromManyArray(array $data): array { diff --git a/app/Exports/ZipExports/Models/ZipExportPage.php b/app/Exports/ZipExports/Models/ZipExportPage.php index 097443df0..6de7f9446 100644 --- a/app/Exports/ZipExports/Models/ZipExportPage.php +++ b/app/Exports/ZipExports/Models/ZipExportPage.php @@ -7,7 +7,7 @@ use BookStack\Entities\Tools\PageContent; use BookStack\Exports\ZipExports\ZipExportFiles; use BookStack\Exports\ZipExports\ZipValidationHelper; -class ZipExportPage extends ZipExportModel +final class ZipExportPage extends ZipExportModel { public ?int $id = null; public string $name; @@ -86,9 +86,9 @@ class ZipExportPage extends ZipExportModel return $errors; } - public static function fromArray(array $data): self + public static function fromArray(array $data): static { - $model = new self(); + $model = new static(); $model->id = $data['id'] ?? null; $model->name = $data['name']; diff --git a/app/Exports/ZipExports/Models/ZipExportTag.php b/app/Exports/ZipExports/Models/ZipExportTag.php index 6b4720fca..8ac7f3c4d 100644 --- a/app/Exports/ZipExports/Models/ZipExportTag.php +++ b/app/Exports/ZipExports/Models/ZipExportTag.php @@ -5,7 +5,7 @@ namespace BookStack\Exports\ZipExports\Models; use BookStack\Activity\Models\Tag; use BookStack\Exports\ZipExports\ZipValidationHelper; -class ZipExportTag extends ZipExportModel +final class ZipExportTag extends ZipExportModel { public string $name; public ?string $value = null; @@ -39,9 +39,9 @@ class ZipExportTag extends ZipExportModel return $context->validateData($data, $rules); } - public static function fromArray(array $data): self + public static function fromArray(array $data): static { - $model = new self(); + $model = new static(); $model->name = $data['name']; $model->value = $data['value'] ?? null; diff --git a/app/Http/Kernel.php b/app/Http/Kernel.php index 30714e2ac..00bf8cbe1 100644 --- a/app/Http/Kernel.php +++ b/app/Http/Kernel.php @@ -9,6 +9,8 @@ class Kernel extends HttpKernel /** * The application's global HTTP middleware stack. * These middleware are run during every request to your application. + * + * @var list */ protected $middleware = [ \BookStack\Http\Middleware\PreventRequestsDuringMaintenance::class, @@ -21,7 +23,7 @@ class Kernel extends HttpKernel /** * The application's route middleware groups. * - * @var array + * @var array> */ protected $middlewareGroups = [ 'web' => [ @@ -47,7 +49,7 @@ class Kernel extends HttpKernel /** * The application's middleware aliases. * - * @var array + * @var array */ protected $middlewareAliases = [ 'auth' => \BookStack\Http\Middleware\Authenticate::class, diff --git a/app/Http/Middleware/EncryptCookies.php b/app/Http/Middleware/EncryptCookies.php index 484c15ae8..2d7d5d9d4 100644 --- a/app/Http/Middleware/EncryptCookies.php +++ b/app/Http/Middleware/EncryptCookies.php @@ -9,7 +9,7 @@ class EncryptCookies extends Middleware /** * The names of the cookies that should not be encrypted. * - * @var array + * @var array */ protected $except = [ // diff --git a/app/Http/Middleware/PreventRequestsDuringMaintenance.php b/app/Http/Middleware/PreventRequestsDuringMaintenance.php index dfb9592e1..c1a02c8cc 100644 --- a/app/Http/Middleware/PreventRequestsDuringMaintenance.php +++ b/app/Http/Middleware/PreventRequestsDuringMaintenance.php @@ -9,7 +9,7 @@ class PreventRequestsDuringMaintenance extends Middleware /** * The URIs that should be reachable while maintenance mode is enabled. * - * @var array + * @var array */ protected $except = [ // diff --git a/app/Http/Middleware/TrimStrings.php b/app/Http/Middleware/TrimStrings.php index cbdc88fb9..ec5725d67 100644 --- a/app/Http/Middleware/TrimStrings.php +++ b/app/Http/Middleware/TrimStrings.php @@ -9,7 +9,7 @@ class TrimStrings extends Middleware /** * The names of the attributes that should not be trimmed. * - * @var array + * @var array */ protected $except = [ 'password', diff --git a/app/Http/Middleware/TrustProxies.php b/app/Http/Middleware/TrustProxies.php index 0fe0247b8..ecf6cb18a 100644 --- a/app/Http/Middleware/TrustProxies.php +++ b/app/Http/Middleware/TrustProxies.php @@ -11,7 +11,7 @@ class TrustProxies extends Middleware /** * The trusted proxies for this application. * - * @var array + * @var array|string|null */ protected $proxies; diff --git a/app/Http/Middleware/VerifyCsrfToken.php b/app/Http/Middleware/VerifyCsrfToken.php index 804a22bc0..7c8689b8b 100644 --- a/app/Http/Middleware/VerifyCsrfToken.php +++ b/app/Http/Middleware/VerifyCsrfToken.php @@ -16,7 +16,7 @@ class VerifyCsrfToken extends Middleware /** * The URIs that should be excluded from CSRF verification. * - * @var array + * @var array */ protected $except = [ 'saml2/*', diff --git a/app/Search/SearchOptionSet.php b/app/Search/SearchOptionSet.php index bd5e5a5b2..844d145e6 100644 --- a/app/Search/SearchOptionSet.php +++ b/app/Search/SearchOptionSet.php @@ -14,6 +14,9 @@ class SearchOptionSet */ protected array $options = []; + /** + * @param T[] $options + */ public function __construct(array $options = []) { $this->options = $options; diff --git a/app/Search/SearchRunner.php b/app/Search/SearchRunner.php index 9716f8053..c14d1c642 100644 --- a/app/Search/SearchRunner.php +++ b/app/Search/SearchRunner.php @@ -285,7 +285,7 @@ class SearchRunner * * @param array $termCounts * - * @return array + * @return array */ protected function rawTermCountsToAdjustments(array $termCounts): array { diff --git a/app/Users/Models/User.php b/app/Users/Models/User.php index b6989ce2d..f83e12088 100644 --- a/app/Users/Models/User.php +++ b/app/Users/Models/User.php @@ -26,6 +26,7 @@ use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Relations\BelongsTo; use Illuminate\Database\Eloquent\Relations\BelongsToMany; use Illuminate\Database\Eloquent\Relations\HasMany; +use Illuminate\Database\Eloquent\Relations\Relation; use Illuminate\Notifications\Notifiable; use Illuminate\Support\Collection; @@ -64,7 +65,7 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon /** * The attributes that are mass assignable. * - * @var array + * @var list */ protected $fillable = ['name', 'email']; @@ -73,7 +74,7 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon /** * The attributes excluded from the model's JSON form. * - * @var array + * @var list */ protected $hidden = [ 'password', 'remember_token', 'system_name', 'email_confirmed', 'external_auth_id', 'email', @@ -118,14 +119,10 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon /** * The roles that belong to the user. * - * @return BelongsToMany + * @return BelongsToMany */ - public function roles() + public function roles(): BelongsToMany { - if ($this->id === 0) { - return; - } - return $this->belongsToMany(Role::class); } diff --git a/app/Util/OutOfMemoryHandler.php b/app/Util/OutOfMemoryHandler.php index 88e9581f4..6632d22a5 100644 --- a/app/Util/OutOfMemoryHandler.php +++ b/app/Util/OutOfMemoryHandler.php @@ -42,7 +42,7 @@ class OutOfMemoryHandler } /** - * Forget the handler so no action is taken place on out of memory. + * Forget the handler, so no action is taken place on out of memory. */ public function forget(): void { @@ -53,6 +53,11 @@ class OutOfMemoryHandler protected function getHandler(): Handler { + /** + * We want to resolve our specific BookStack handling via the set app handler + * singleton, but phpstan will only infer based on the interface. + * @phpstan-ignore return.type + */ return app()->make(ExceptionHandler::class); } } From 318b486e0b7601a00dc02bd1adcef22066bfef5d Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Wed, 3 Sep 2025 15:18:49 +0100 Subject: [PATCH 4/5] Maintenance: Finished changes to meet phpstan level 3 --- .../Guards/AsyncExternalBaseSessionGuard.php | 20 ++++-------- app/Access/Guards/LdapSessionGuard.php | 6 +--- app/Access/LoginService.php | 10 +++--- app/Entities/Queries/BookQueries.php | 3 ++ app/Entities/Queries/BookshelfQueries.php | 3 ++ app/Entities/Queries/ChapterQueries.php | 3 ++ app/Entities/Queries/PageQueries.php | 6 ++++ .../Queries/ProvidesEntityQueries.php | 7 +++-- app/Entities/Tools/PageIncludeParser.php | 6 ++-- app/Exceptions/Handler.php | 31 +++++++------------ app/Permissions/EntityPermissionEvaluator.php | 4 +-- 11 files changed, 48 insertions(+), 51 deletions(-) diff --git a/app/Access/Guards/AsyncExternalBaseSessionGuard.php b/app/Access/Guards/AsyncExternalBaseSessionGuard.php index ab982a6b0..b66fbe95e 100644 --- a/app/Access/Guards/AsyncExternalBaseSessionGuard.php +++ b/app/Access/Guards/AsyncExternalBaseSessionGuard.php @@ -3,23 +3,18 @@ namespace BookStack\Access\Guards; /** - * Saml2 Session Guard. + * External Auth Session Guard. * - * The saml2 login process is async in nature meaning it does not fit very well - * into the default laravel 'Guard' auth flow. Instead most of the logic is done - * via the Saml2 controller & Saml2Service. This class provides a safer, thin - * version of SessionGuard. + * The login process for external auth (SAML2/OIDC) is async in nature, meaning it does not fit very well + * into the default laravel 'Guard' auth flow. Instead, most of the logic is done via the relevant + * controller and services. This class provides a safer, thin version of SessionGuard. */ class AsyncExternalBaseSessionGuard extends ExternalBaseSessionGuard { /** * Validate a user's credentials. - * - * @param array $credentials - * - * @return bool */ - public function validate(array $credentials = []) + public function validate(array $credentials = []): bool { return false; } @@ -27,12 +22,9 @@ class AsyncExternalBaseSessionGuard extends ExternalBaseSessionGuard /** * Attempt to authenticate a user using the given credentials. * - * @param array $credentials * @param bool $remember - * - * @return bool */ - public function attempt(array $credentials = [], $remember = false) + public function attempt(array $credentials = [], $remember = false): bool { return false; } diff --git a/app/Access/Guards/LdapSessionGuard.php b/app/Access/Guards/LdapSessionGuard.php index bd020f70b..9455d530d 100644 --- a/app/Access/Guards/LdapSessionGuard.php +++ b/app/Access/Guards/LdapSessionGuard.php @@ -35,13 +35,9 @@ class LdapSessionGuard extends ExternalBaseSessionGuard /** * Validate a user's credentials. * - * @param array $credentials - * * @throws LdapException - * - * @return bool */ - public function validate(array $credentials = []) + public function validate(array $credentials = []): bool { $userDetails = $this->ldapService->getUserDetails($credentials['username']); diff --git a/app/Access/LoginService.php b/app/Access/LoginService.php index 6607969af..76ae1938d 100644 --- a/app/Access/LoginService.php +++ b/app/Access/LoginService.php @@ -95,7 +95,7 @@ class LoginService { $value = session()->get(self::LAST_LOGIN_ATTEMPTED_SESSION_KEY); if (!$value) { - return ['user_id' => null, 'method' => null]; + return ['user_id' => null, 'method' => null, 'remember' => false]; } [$id, $method, $remember, $time] = explode(':', $value); @@ -103,18 +103,18 @@ class LoginService if ($time < $hourAgo) { $this->clearLastLoginAttempted(); - return ['user_id' => null, 'method' => null]; + return ['user_id' => null, 'method' => null, 'remember' => false]; } return ['user_id' => $id, 'method' => $method, 'remember' => boolval($remember)]; } /** - * Set the last login attempted user. + * Set the last login-attempted user. * Must be only used when credentials are correct and a login could be - * achieved but a secondary factor has stopped the login. + * achieved, but a secondary factor has stopped the login. */ - protected function setLastLoginAttemptedForUser(User $user, string $method, bool $remember) + protected function setLastLoginAttemptedForUser(User $user, string $method, bool $remember): void { session()->put( self::LAST_LOGIN_ATTEMPTED_SESSION_KEY, diff --git a/app/Entities/Queries/BookQueries.php b/app/Entities/Queries/BookQueries.php index 5ff22252c..2492f8131 100644 --- a/app/Entities/Queries/BookQueries.php +++ b/app/Entities/Queries/BookQueries.php @@ -6,6 +6,9 @@ use BookStack\Entities\Models\Book; use BookStack\Exceptions\NotFoundException; use Illuminate\Database\Eloquent\Builder; +/** + * @implements ProvidesEntityQueries + */ class BookQueries implements ProvidesEntityQueries { protected static array $listAttributes = [ diff --git a/app/Entities/Queries/BookshelfQueries.php b/app/Entities/Queries/BookshelfQueries.php index 874bc7f2f..842011a87 100644 --- a/app/Entities/Queries/BookshelfQueries.php +++ b/app/Entities/Queries/BookshelfQueries.php @@ -6,6 +6,9 @@ use BookStack\Entities\Models\Bookshelf; use BookStack\Exceptions\NotFoundException; use Illuminate\Database\Eloquent\Builder; +/** + * @implements ProvidesEntityQueries + */ class BookshelfQueries implements ProvidesEntityQueries { protected static array $listAttributes = [ diff --git a/app/Entities/Queries/ChapterQueries.php b/app/Entities/Queries/ChapterQueries.php index 53c5bc9d8..9bf0ff65b 100644 --- a/app/Entities/Queries/ChapterQueries.php +++ b/app/Entities/Queries/ChapterQueries.php @@ -6,6 +6,9 @@ use BookStack\Entities\Models\Chapter; use BookStack\Exceptions\NotFoundException; use Illuminate\Database\Eloquent\Builder; +/** + * @implements ProvidesEntityQueries + */ class ChapterQueries implements ProvidesEntityQueries { protected static array $listAttributes = [ diff --git a/app/Entities/Queries/PageQueries.php b/app/Entities/Queries/PageQueries.php index f821ee86a..ee7b201bc 100644 --- a/app/Entities/Queries/PageQueries.php +++ b/app/Entities/Queries/PageQueries.php @@ -6,6 +6,9 @@ use BookStack\Entities\Models\Page; use BookStack\Exceptions\NotFoundException; use Illuminate\Database\Eloquent\Builder; +/** + * @implements ProvidesEntityQueries + */ class PageQueries implements ProvidesEntityQueries { protected static array $contentAttributes = [ @@ -18,6 +21,9 @@ class PageQueries implements ProvidesEntityQueries 'template', 'text', 'created_at', 'updated_at', 'priority', 'owned_by', ]; + /** + * @return Builder + */ public function start(): Builder { return Page::query(); diff --git a/app/Entities/Queries/ProvidesEntityQueries.php b/app/Entities/Queries/ProvidesEntityQueries.php index 1f8a71ae2..79fc64b3a 100644 --- a/app/Entities/Queries/ProvidesEntityQueries.php +++ b/app/Entities/Queries/ProvidesEntityQueries.php @@ -7,17 +7,20 @@ use Illuminate\Database\Eloquent\Builder; /** * Interface for our classes which provide common queries for our - * entity objects. Ideally all queries for entities should run through + * entity objects. Ideally, all queries for entities should run through * these classes. * Any added methods should return a builder instances to allow extension * via building on the query, unless the method starts with 'find' * in which case an entity object should be returned. * (nullable unless it's a *OrFail method). + * + * @template TModel of Entity */ interface ProvidesEntityQueries { /** * Start a new query for this entity type. + * @return Builder */ public function start(): Builder; @@ -29,7 +32,7 @@ interface ProvidesEntityQueries /** * Start a query for items that are visible, with selection * configured for list display of this item. - * @return Builder + * @return Builder */ public function visibleForList(): Builder; } diff --git a/app/Entities/Tools/PageIncludeParser.php b/app/Entities/Tools/PageIncludeParser.php index 329a7633f..af7ed4fc6 100644 --- a/app/Entities/Tools/PageIncludeParser.php +++ b/app/Entities/Tools/PageIncludeParser.php @@ -13,8 +13,8 @@ class PageIncludeParser protected static string $includeTagRegex = "/{{@\s?([0-9].*?)}}/"; /** - * Elements to clean up and remove if left empty after a parsing operation. - * @var DOMElement[] + * Nodes to clean up and remove if left empty after a parsing operation. + * @var DOMNode[] */ protected array $toCleanup = []; @@ -206,7 +206,7 @@ class PageIncludeParser } /** - * Cleanup after a parse operation. + * Clean up after a parse operation. * Removes stranded elements we may have left during the parse. */ protected function cleanup(): void diff --git a/app/Exceptions/Handler.php b/app/Exceptions/Handler.php index b8c3fa192..08d326ad8 100644 --- a/app/Exceptions/Handler.php +++ b/app/Exceptions/Handler.php @@ -2,7 +2,6 @@ namespace BookStack\Exceptions; -use Exception; use Illuminate\Auth\AuthenticationException; use Illuminate\Database\Eloquent\ModelNotFoundException; use Illuminate\Foundation\Exceptions\Handler as ExceptionHandler; @@ -12,6 +11,7 @@ use Illuminate\Http\Request; use Illuminate\Http\Response; use Illuminate\Validation\ValidationException; use Symfony\Component\ErrorHandler\Error\FatalError; +use Symfony\Component\HttpFoundation\Response as SymfonyResponse; use Symfony\Component\HttpKernel\Exception\HttpExceptionInterface; use Throwable; @@ -20,7 +20,7 @@ class Handler extends ExceptionHandler /** * A list of the exception types that are not reported. * - * @var array> + * @var array> */ protected $dontReport = [ NotFoundException::class, @@ -50,11 +50,11 @@ class Handler extends ExceptionHandler /** * Report or log an exception. * - * @param \Throwable $exception - * - * @throws \Throwable + * @param Throwable $exception * * @return void + *@throws Throwable + * */ public function report(Throwable $exception) { @@ -64,12 +64,9 @@ class Handler extends ExceptionHandler /** * Render an exception into an HTTP response. * - * @param \Illuminate\Http\Request $request - * @param Exception $e - * - * @return \Illuminate\Http\Response + * @param Request $request */ - public function render($request, Throwable $e) + public function render($request, Throwable $e): SymfonyResponse { if ($e instanceof FatalError && str_contains($e->getMessage(), 'bytes exhausted (tried to allocate') && $this->onOutOfMemory) { $response = call_user_func($this->onOutOfMemory); @@ -152,12 +149,9 @@ class Handler extends ExceptionHandler /** * Convert an authentication exception into an unauthenticated response. * - * @param \Illuminate\Http\Request $request - * @param \Illuminate\Auth\AuthenticationException $exception - * - * @return \Illuminate\Http\Response + * @param Request $request */ - protected function unauthenticated($request, AuthenticationException $exception) + protected function unauthenticated($request, AuthenticationException $exception): SymfonyResponse { if ($request->expectsJson()) { return response()->json(['error' => 'Unauthenticated.'], 401); @@ -169,12 +163,9 @@ class Handler extends ExceptionHandler /** * Convert a validation exception into a JSON response. * - * @param \Illuminate\Http\Request $request - * @param \Illuminate\Validation\ValidationException $exception - * - * @return \Illuminate\Http\JsonResponse + * @param Request $request */ - protected function invalidJson($request, ValidationException $exception) + protected function invalidJson($request, ValidationException $exception): JsonResponse { return response()->json($exception->errors(), $exception->status); } diff --git a/app/Permissions/EntityPermissionEvaluator.php b/app/Permissions/EntityPermissionEvaluator.php index 98ec03306..30c97b700 100644 --- a/app/Permissions/EntityPermissionEvaluator.php +++ b/app/Permissions/EntityPermissionEvaluator.php @@ -30,7 +30,7 @@ class EntityPermissionEvaluator } /** - * @param array> $permitsByType + * @param array> $permitsByType */ protected function evaluatePermitsByType(array $permitsByType): ?int { @@ -50,7 +50,7 @@ class EntityPermissionEvaluator /** * @param string[] $typeIdChain * @param array $permissionMapByTypeId - * @return array> + * @return array> */ protected function collapseAndCategorisePermissions(array $typeIdChain, array $permissionMapByTypeId): array { From 7d1c31620213599fdc07ce5cda776d91f5642afc Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Wed, 3 Sep 2025 15:42:50 +0100 Subject: [PATCH 5/5] Maintenance: Updated larastan target level, fixed issues from tests --- app/Access/Guards/ExternalBaseSessionGuard.php | 2 +- app/Console/Commands/UpdateUrlCommand.php | 2 +- app/Entities/Controllers/RecycleBinApiController.php | 7 ++++--- phpstan.neon.dist | 2 +- tests/Entity/CommentStoreTest.php | 1 - 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/Access/Guards/ExternalBaseSessionGuard.php b/app/Access/Guards/ExternalBaseSessionGuard.php index 899e93c36..91239599b 100644 --- a/app/Access/Guards/ExternalBaseSessionGuard.php +++ b/app/Access/Guards/ExternalBaseSessionGuard.php @@ -30,7 +30,7 @@ class ExternalBaseSessionGuard implements StatefulGuard /** * The user we last attempted to retrieve. */ - protected Authenticatable $lastAttempted; + protected Authenticatable|null $lastAttempted; /** * The session used by the guard. diff --git a/app/Console/Commands/UpdateUrlCommand.php b/app/Console/Commands/UpdateUrlCommand.php index e155878d3..71f0b92fe 100644 --- a/app/Console/Commands/UpdateUrlCommand.php +++ b/app/Console/Commands/UpdateUrlCommand.php @@ -52,7 +52,7 @@ class UpdateUrlCommand extends Command 'page_revisions' => ['html', 'text', 'markdown'], 'images' => ['url'], 'settings' => ['value'], - 'comments' => ['html', 'text'], + 'comments' => ['html'], ]; foreach ($columnsToUpdateByTable as $table => $columns) { diff --git a/app/Entities/Controllers/RecycleBinApiController.php b/app/Entities/Controllers/RecycleBinApiController.php index fdc24ddf8..89bd3f41a 100644 --- a/app/Entities/Controllers/RecycleBinApiController.php +++ b/app/Entities/Controllers/RecycleBinApiController.php @@ -9,6 +9,7 @@ use BookStack\Entities\Models\Deletion; use BookStack\Entities\Models\Page; use BookStack\Entities\Repos\DeletionRepo; use BookStack\Http\ApiController; +use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Relations\HasMany; class RecycleBinApiController extends ApiController @@ -69,7 +70,7 @@ class RecycleBinApiController extends ApiController /** * Load some related details for the deletion listing. */ - protected function listFormatter(Deletion $deletion) + protected function listFormatter(Deletion $deletion): void { $deletable = $deletion->deletable; @@ -89,9 +90,9 @@ class RecycleBinApiController extends ApiController } /** - * @param HasMany $query + * @param Builder $query */ - protected static function withTrashedQuery(HasMany $query): void + protected static function withTrashedQuery(Builder $query): void { $query->withTrashed(); } diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 9dfd9d29e..72189222f 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -7,7 +7,7 @@ parameters: - app # The level 8 is the highest level - level: 2 + level: 3 phpVersion: min: 80200 diff --git a/tests/Entity/CommentStoreTest.php b/tests/Entity/CommentStoreTest.php index c5fe4ce50..de093a3a6 100644 --- a/tests/Entity/CommentStoreTest.php +++ b/tests/Entity/CommentStoreTest.php @@ -27,7 +27,6 @@ class CommentStoreTest extends TestCase 'local_id' => 1, 'entity_id' => $page->id, 'entity_type' => Page::newModelInstance()->getMorphClass(), - 'text' => null, 'parent_id' => 2, ]);