feat(auth/oidc): wider key algorithm support

.env.example

@@ -26,13 +26,6 @@ DB_DATABASE=database_database
 DB_USERNAME=database_username
 DB_PASSWORD=database_user_password
 
-# Storage system to use
-# By default files are stored on the local filesystem, with images being placed in
-# public web space so they can be efficiently served directly by the web-server.
-# For other options with different security levels & considerations, refer to:
-# https://www.bookstackapp.com/docs/admin/upload-config/
-STORAGE_TYPE=local
-
 # Mail system to use
 # Can be 'smtp' or 'sendmail'
 MAIL_DRIVER=smtp

.github/translators.txt

@@ -511,4 +511,3 @@ MrCharlesIII :: Arabic
 David Olsen (dawin) :: Danish
 ltnzr :: French
 Frank Holler (holler.frank) :: German; German Informal
-Korab Arifi (korabidev) :: Albanian

.github/workflows/test-migrations.yml

@@ -16,7 +16,7 @@ jobs:
     runs-on: ubuntu-24.04
     strategy:
       matrix:
-        php: ['8.2', '8.3', '8.4', '8.5']
+        php: ['8.2', '8.3', '8.4']
     steps:
       - uses: actions/checkout@v4
 

.github/workflows/test-php.yml

@@ -16,7 +16,7 @@ jobs:
     runs-on: ubuntu-24.04
     strategy:
       matrix:
-        php: ['8.2', '8.3', '8.4', '8.5']
+        php: ['8.2', '8.3', '8.4']
     steps:
     - uses: actions/checkout@v4
 

.gitignore

@@ -8,10 +8,10 @@ Homestead.yaml
 .idea
 npm-debug.log
 yarn-error.log
-/public/dist/*.map
+/public/dist
 /public/plugins
-/public/css/*.map
-/public/js/*.map
+/public/css
+/public/js
 /public/bower
 /public/build/
 /public/favicon.ico

app/Access/Controllers/OidcController.php

@@ -9,9 +9,11 @@ use Illuminate\Http\Request;
 
 class OidcController extends Controller
 {
-    public function __construct(
-        protected OidcService $oidcService
-    ) {
+    protected OidcService $oidcService;
+
+    public function __construct(OidcService $oidcService)
+    {
+        $this->oidcService = $oidcService;
         $this->middleware('guard:oidc');
     }
 
@@ -28,7 +30,7 @@ class OidcController extends Controller
             return redirect('/login');
         }
 
-        session()->put('oidc_state', time() . ':' . $loginDetails['state']);
+        session()->flash('oidc_state', $loginDetails['state']);
 
         return redirect($loginDetails['url']);
     }
@@ -39,16 +41,10 @@ class OidcController extends Controller
      */
     public function callback(Request $request)
     {
+        $storedState = session()->pull('oidc_state');
         $responseState = $request->query('state');
-        $splitState =  explode(':', session()->pull('oidc_state', ':'), 2);
-        if (count($splitState) !== 2) {
-            $splitState = [null, null];
-        }
 
-        [$storedStateTime, $storedState] = $splitState;
-        $threeMinutesAgo = time() - 3 * 60;
-
-        if (!$storedState || $storedState !== $responseState || intval($storedStateTime) < $threeMinutesAgo) {
+        if ($storedState !== $responseState) {
             $this->showErrorNotification(trans('errors.oidc_fail_authed', ['system' => config('oidc.name')]));
 
             return redirect('/login');
@@ -66,7 +62,7 @@ class OidcController extends Controller
     }
 
     /**
-     * Log the user out, then start the OIDC RP-initiated logout process.
+     * Log the user out then start the OIDC RP-initiated logout process.
      */
     public function logout()
     {

app/Access/Oidc/OidcJwtSigningKey.php

@@ -2,6 +2,7 @@
 
 namespace BookStack\Access\Oidc;
 
+use Illuminate\Support\Facades\Log;
 use phpseclib3\Crypt\Common\PublicKey;
 use phpseclib3\Crypt\PublicKeyLoader;
 use phpseclib3\Crypt\RSA;
@@ -63,8 +64,9 @@ class OidcJwtSigningKey
         // 'alg' is optional for a JWK, but we will still attempt to validate if
         // it exists otherwise presume it will be compatible.
         $alg = $jwk['alg'] ?? null;
-        if ($jwk['kty'] !== 'RSA' || !(is_null($alg) || $alg === 'RS256')) {
-            throw new OidcInvalidKeyException("Only RS256 keys are currently supported. Found key using {$alg}");
+        $algorithm = OidcJwtSigningKeyAlgorithm::tryFrom($alg ?? OidcJwtSigningKeyAlgorithm::RS256->value);
+        if ($jwk['kty'] !== 'RSA' || $algorithm === null) {
+            throw new OidcInvalidKeyException("Only " . OidcJwtSigningKeyAlgorithm::getSupportedAlgorithms() . " keys are currently supported. Found key using {$alg}");
         }
 
         // 'use' is optional for a JWK but we assume 'sig' where no value exists since that's what
@@ -97,7 +99,16 @@ class OidcJwtSigningKey
             throw new OidcInvalidKeyException('Key loaded from file path is not an RSA key as expected');
         }
 
-        $this->key = $key->withPadding(RSA::SIGNATURE_PKCS1);
+        // apply key-algorithm depending hash
+        $key = match ($algorithm) {
+            OidcJwtSigningKeyAlgorithm::RS256 => $key->withHash('sha256'),
+            OidcJwtSigningKeyAlgorithm::RS512 => $key->withHash('sha512'),
+        };
+        // apply key-algorithm depending padding
+        $this->key = match ($algorithm) {
+            OidcJwtSigningKeyAlgorithm::RS256,
+            OidcJwtSigningKeyAlgorithm::RS512 => $key->withPadding(RSA::SIGNATURE_PKCS1),
+        };
     }
 
     /**

app/Access/Oidc/OidcJwtSigningKeyAlgorithm.php

@@ -0,0 +1,16 @@
+<?php
+
+namespace BookStack\Access\Oidc;
+
+use UnitEnum;
+
+enum OidcJwtSigningKeyAlgorithm: string
+{
+    case RS256 = 'RS256';
+    case RS512 = 'RS512';
+
+    public static function getSupportedAlgorithms(): string
+    {
+        return join(',', array_map(static fn (UnitEnum $enum) => $enum->value, self::cases()));
+    }
+}

app/Access/Oidc/OidcJwtWithClaims.php

@@ -119,8 +119,8 @@ class OidcJwtWithClaims implements ProvidesClaims
      */
     protected function validateTokenSignature(): void
     {
-        if ($this->header['alg'] !== 'RS256') {
-            throw new OidcInvalidTokenException("Only RS256 signature validation is supported. Token reports using {$this->header['alg']}");
+        if (OidcJwtSigningKeyAlgorithm::tryFrom($this->header['alg']) === null) {
+            throw new OidcInvalidTokenException("Only " . OidcJwtSigningKeyAlgorithm::getSupportedAlgorithms() . " signature validation is supported. Token reports using {$this->header['alg']}");
         }
 
         $parsedKeys = array_map(function ($key) {

app/Access/Oidc/OidcProviderSettings.php

@@ -158,10 +158,10 @@ class OidcProviderSettings
     protected function filterKeys(array $keys): array
     {
         return array_filter($keys, function (array $key) {
-            $alg = $key['alg'] ?? 'RS256';
+            $alg = $key['alg'] ?? OidcJwtSigningKeyAlgorithm::RS256->value;
             $use = $key['use'] ?? 'sig';
 
-            return $key['kty'] === 'RSA' && $use === 'sig' && $alg === 'RS256';
+            return $key['kty'] === 'RSA' && $use === 'sig' && OidcJwtSigningKeyAlgorithm::tryFrom($alg) !== null;
         });
     }
 

app/Activity/Models/Comment.php

@@ -41,19 +41,7 @@ class Comment extends Model implements Loggable, OwnableInterface
      */
     public function entity(): MorphTo
     {
-        // We specifically define null here to avoid the different name (commentable)
-        // being used by Laravel eager loading instead of the method name, which it was doing
-        // in some scenarios like when deserialized when going through the queue system.
-        // So we instead specify the type and id column names to use.
-        // Related to:
-        // https://github.com/laravel/framework/pull/24815
-        // https://github.com/laravel/framework/issues/27342
-        // https://github.com/laravel/framework/issues/47953
-        // (and probably more)
-
-        // Ultimately, we could just align the method name to 'commentable' but that would be a potential
-        // breaking change and not really worthwhile in a patch due to the risk of creating extra problems.
-        return $this->morphTo(null, 'commentable_type', 'commentable_id');
+        return $this->morphTo('commentable');
     }
 
     /**

app/Activity/Notifications/Handlers/BaseNotificationHandler.php

@@ -20,7 +20,6 @@ abstract class BaseNotificationHandler implements NotificationHandler
     {
         $users = User::query()->whereIn('id', array_unique($userIds))->get();
 
-        /** @var User $user */
         foreach ($users as $user) {
             // Prevent sending to the user that initiated the activity
             if ($user->id === $initiator->id) {

app/Entities/Models/Book.php

@@ -67,7 +67,8 @@ class Book extends Entity implements HasDescriptionInterface, HasCoverInterface,
      */
     public function chapters(): HasMany
     {
-        return $this->hasMany(Chapter::class);
+        return $this->hasMany(Chapter::class)
+            ->where('type', '=', 'chapter');
     }
 
     /**

app/Entities/Models/EntityScope.php

@@ -15,12 +15,11 @@ class EntityScope implements Scope
     public function apply(Builder $builder, Model $model): void
     {
         $builder = $builder->where('type', '=', $model->getMorphClass());
-        $table = $model->getTable();
         if ($model instanceof Page) {
-            $builder->leftJoin('entity_page_data', 'entity_page_data.page_id', '=', "{$table}.id");
+            $builder->leftJoin('entity_page_data', 'entity_page_data.page_id', '=', 'entities.id');
         } else {
-            $builder->leftJoin('entity_container_data', function (JoinClause $join) use ($model, $table) {
-                $join->on('entity_container_data.entity_id', '=', "{$table}.id")
+            $builder->leftJoin('entity_container_data', function (JoinClause $join) use ($model) {
+                $join->on('entity_container_data.entity_id', '=', 'entities.id')
                     ->where('entity_container_data.entity_type', '=', $model->getMorphClass());
             });
         }

app/Exports/ZipExports/ZipExportReferences.php

@@ -15,7 +15,6 @@ use BookStack\Exports\ZipExports\Models\ZipExportPage;
 use BookStack\Permissions\Permission;
 use BookStack\Uploads\Attachment;
 use BookStack\Uploads\Image;
-use BookStack\Uploads\ImageService;
 
 class ZipExportReferences
 {
@@ -34,7 +33,6 @@ class ZipExportReferences
 
     public function __construct(
         protected ZipReferenceParser $parser,
-        protected ImageService $imageService,
     ) {
     }
 
@@ -135,17 +133,10 @@ class ZipExportReferences
                 return "[[bsexport:image:{$model->id}]]";
             }
 
-            // Get the page which we'll reference this image upon
+            // Find and include images if in visibility
             $page = $model->getPage();
-            $pageExportModel = null;
-            if ($page && isset($this->pages[$page->id])) {
-                $pageExportModel = $this->pages[$page->id];
-            } elseif ($exportModel instanceof ZipExportPage) {
-                $pageExportModel = $exportModel;
-            }
-
-            // Add the image to the export if it's accessible or just return the existing reference if already added
-            if (isset($this->images[$model->id]) || ($pageExportModel && $this->imageService->imageAccessible($model))) {
+            $pageExportModel = $this->pages[$page->id] ?? ($exportModel instanceof ZipExportPage ? $exportModel : null);
+            if (isset($this->images[$model->id]) || ($page && $pageExportModel && userCan(Permission::PageView, $page))) {
                 if (!isset($this->images[$model->id])) {
                     $exportImage = ZipExportImage::fromModel($model, $files);
                     $this->images[$model->id] = $exportImage;
@@ -153,7 +144,6 @@ class ZipExportReferences
                 }
                 return "[[bsexport:image:{$model->id}]]";
             }
-
             return null;
         }
 

app/Http/Middleware/StartSessionExtended.php

@@ -14,10 +14,7 @@ use Illuminate\Session\Middleware\StartSession as Middleware;
 class StartSessionExtended extends Middleware
 {
     protected static array $pathPrefixesExcludedFromHistory = [
-        'uploads/images/',
-        'dist/',
-        'manifest.json',
-        'opensearch.xml',
+        'uploads/images/'
     ];
 
     /**

app/Search/SearchIndex.php

@@ -126,7 +126,7 @@ class SearchIndex
         $termMap = $this->textToTermCountMap($text);
 
         foreach ($termMap as $term => $count) {
-            $termMap[$term] = intval($count * $scoreAdjustment);
+            $termMap[$term] = floor($count * $scoreAdjustment);
         }
 
         return $termMap;

app/Uploads/Image.php

@@ -13,14 +13,14 @@ use Illuminate\Database\Eloquent\Factories\HasFactory;
 use Illuminate\Database\Eloquent\Relations\HasMany;
 
 /**
- * @property int      $id
- * @property string   $name
- * @property string   $url
- * @property string   $path
- * @property string   $type
- * @property int|null $uploaded_to
- * @property int      $created_by
- * @property int      $updated_by
+ * @property int    $id
+ * @property string $name
+ * @property string $url
+ * @property string $path
+ * @property string $type
+ * @property int    $uploaded_to
+ * @property int    $created_by
+ * @property int    $updated_by
  */
 class Image extends Model implements OwnableInterface
 {

app/Uploads/ImageService.php

@@ -148,7 +148,7 @@ class ImageService
     }
 
     /**
-     * Destroy an image along with its revisions, thumbnails, and remaining folders.
+     * Destroy an image along with its revisions, thumbnails and remaining folders.
      *
      * @throws Exception
      */
@@ -252,7 +252,16 @@ class ImageService
     {
         $disk = $this->storage->getDisk('gallery');
 
-        return $disk->usingSecureImages() && $this->pathAccessible($imagePath);
+        if ($this->storage->usingSecureRestrictedImages() && !$this->checkUserHasAccessToRelationOfImageAtPath($imagePath)) {
+            return false;
+        }
+
+        // Check local_secure is active
+        return $disk->usingSecureImages()
+            // Check the image file exists
+            && $disk->exists($imagePath)
+            // Check the file is likely an image file
+            && str_starts_with($disk->mimeType($imagePath), 'image/');
     }
 
     /**
@@ -260,51 +269,16 @@ class ImageService
      */
     public function pathAccessible(string $imagePath): bool
     {
+        $disk = $this->storage->getDisk('gallery');
+
         if ($this->storage->usingSecureRestrictedImages() && !$this->checkUserHasAccessToRelationOfImageAtPath($imagePath)) {
             return false;
         }
 
-        if ($this->blockedBySecureImages()) {
-            return false;
-        }
-
-        return $this->imageFileExists($imagePath, 'gallery');
-    }
-
-    /**
-     * Check if the given image should be accessible to the current user.
-     */
-    public function imageAccessible(Image $image): bool
-    {
-        if ($this->storage->usingSecureRestrictedImages() && !$this->checkUserHasAccessToRelationOfImage($image)) {
-            return false;
-        }
-
-        if ($this->blockedBySecureImages()) {
-            return false;
-        }
-
-        return $this->imageFileExists($image->path, $image->type);
-    }
-
-    /**
-     * Check if the current user should be blocked from accessing images based on if secure images are enabled
-     * and if public access is enabled for the application.
-     */
-    protected function blockedBySecureImages(): bool
-    {
-        $enforced = $this->storage->usingSecureImages() && !setting('app-public');
-
-        return $enforced && user()->isGuest();
-    }
-
-    /**
-     * Check if the given image path exists for the given image type and that it is likely an image file.
-     */
-    protected function imageFileExists(string $imagePath, string $imageType): bool
-    {
-        $disk = $this->storage->getDisk($imageType);
-        return $disk->exists($imagePath) && str_starts_with($disk->mimeType($imagePath), 'image/');
+        // Check local_secure is active
+        return $disk->exists($imagePath)
+            // Check the file is likely an image file
+            && str_starts_with($disk->mimeType($imagePath), 'image/');
     }
 
     /**
@@ -333,11 +307,6 @@ class ImageService
             return false;
         }
 
-        return $this->checkUserHasAccessToRelationOfImage($image);
-    }
-
-    protected function checkUserHasAccessToRelationOfImage(Image $image): bool
-    {
         $imageType = $image->type;
 
         // Allow user or system (logo) images

app/Uploads/ImageStorage.php

@@ -34,15 +34,6 @@ class ImageStorage
         return config('filesystems.images') === 'local_secure_restricted';
     }
 
-    /**
-     * Check if "local secure" (Fetched behind auth, either with or without permissions enforced)
-     * is currently active in the instance.
-     */
-    public function usingSecureImages(): bool
-    {
-        return config('filesystems.images') === 'local_secure' || $this->usingSecureRestrictedImages();
-    }
-
     /**
      * Clean up an image file name to be both URL and storage safe.
      */
@@ -74,7 +65,7 @@ class ImageStorage
             return 'local';
         }
 
-        // Rename local_secure options to get our image-specific storage driver, which
+        // Rename local_secure options to get our image specific storage driver which
         // is scoped to the relevant image directories.
         if ($localSecureInUse) {
             return 'local_secure_images';

composer.json

@@ -93,7 +93,6 @@
             "@php artisan view:clear"
         ],
         "refresh-test-database": [
-            "@putenv APP_TIMEZONE=UTC",
             "@php artisan migrate:refresh --database=mysql_testing",
             "@php artisan db:seed --class=DummyContentSeeder --database=mysql_testing"
         ]

database/seeders/DummyContentSeeder.php

@@ -13,6 +13,7 @@ use BookStack\Search\SearchIndex;
 use BookStack\Users\Models\Role;
 use BookStack\Users\Models\User;
 use Illuminate\Database\Eloquent\Relations\HasMany;
+use Illuminate\Database\Eloquent\Relations\Relation;
 use Illuminate\Database\Seeder;
 use Illuminate\Support\Collection;
 use Illuminate\Support\Facades\Hash;
@@ -22,8 +23,10 @@ class DummyContentSeeder extends Seeder
 {
     /**
      * Run the database seeds.
+     *
+     * @return void
      */
-    public function run(): void
+    public function run()
     {
         // Create an editor user
         $editorUser = User::factory()->create();

dev/checksums/vendor

@@ -1 +1 @@
-f685179ac59c80d2b635c1df33974830616d7e055fc390be1a17056443644e4a
+22e02ee72d21ff719c1073abbec8302f8e2096ba6d072e133051064ed24b45b1

dev/docker/db-testing/Dockerfile

@@ -1,30 +0,0 @@
-FROM ubuntu:24.04
-
-# Install additional dependencies
-RUN apt-get update && \
-    apt-get install -y \
-        git \
-		wget \
-        zip \
-        unzip \
-		php \
-		php-bcmath php-curl php-mbstring php-gd php-xml php-zip php-mysql php-ldap \
-       && \
-    rm -rf /var/lib/apt/lists/*
-
-# Take branch as an argument so we can choose which BookStack
-# branch to test against
-ARG BRANCH=development
-
-# Download BookStack & install PHP deps
-RUN mkdir -p /var/www && \
-    git clone https://github.com/bookstackapp/bookstack.git --branch "$BRANCH" --single-branch /var/www/bookstack && \
-    cd /var/www/bookstack && \	
-   wget https://raw.githubusercontent.com/composer/getcomposer.org/f3108f64b4e1c1ce6eb462b159956461592b3e3e/web/installer -O - -q | php -- --quiet --filename=composer && \
-    php composer install
-
-# Set the BookStack dir as the default working dir
-WORKDIR /var/www/bookstack
-
-# Set the default action as running php
-ENTRYPOINT ["/bin/php"]

dev/docker/db-testing/run.sh

@@ -1,57 +0,0 @@
-#!/bin/bash
-
-BRANCH=${1:-development}
-
-# Build the container with a known name
-docker build --build-arg BRANCH="$BRANCH" -t bookstack:db-testing .
-if [ $? -eq 1 ]; then
-  echo "Failed to build app container for testing"
-  exit 1
-fi
-
-# List of database containers to test against
-containers=(
-  "mysql:5.7"
-  "mysql:8.0"
-  "mysql:8.4"
-  "mysql:9.5"
-  "mariadb:10.2"
-  "mariadb:10.6"
-  "mariadb:10.11"
-  "mariadb:11.4"
-  "mariadb:11.8"
-  "mariadb:12.0"
-)
-
-# Pre-clean-up from prior runs
-docker stop bs-dbtest-db
-docker network rm bs-dbtest-net
-
-# Cycle over each database image
-for img in "${containers[@]}"; do
-  echo "Starting tests with $img..."
-  docker network create bs-dbtest-net
-  docker run -d --rm --name "bs-dbtest-db" \
-    -e MYSQL_DATABASE=bookstack-test \
-    -e MYSQL_USER=bookstack \
-    -e MYSQL_PASSWORD=bookstack \
-    -e MYSQL_ROOT_PASSWORD=password \
-	  --network=bs-dbtest-net \
-    "$img"
-  sleep 20
-  APP_RUN='docker run -it --rm --network=bs-dbtest-net -e TEST_DATABASE_URL="mysql://bookstack:bookstack@bs-dbtest-db:3306" bookstack:db-testing'
-  $APP_RUN artisan migrate --force --database=mysql_testing
-  $APP_RUN artisan db:seed --force --class=DummyContentSeeder --database=mysql_testing
-  $APP_RUN vendor/bin/phpunit
-  if [ $? -eq 0 ]; then
-    echo "$img - Success"
-  else
-    echo "$img - Error"
-    read -p "Stop script? [y/N] " ans
-    [[ $ans == [yY] ]] && exit 0
-  fi
-
-  docker stop "bs-dbtest-db"
-  docker network rm bs-dbtest-net
-done
-

dev/docs/development.md

@@ -7,7 +7,7 @@ When it's time for a release the `development` branch is merged into release wit
 
 ## Building CSS & JavaScript Assets
 
-This project uses SASS for CSS development which is built, along with the JavaScript, using a range of npm scripts. The below npm commands can be used to install the dependencies & run the build tasks:
+This project uses SASS for CSS development and this is built, along with the JavaScript, using a range of npm scripts. The below npm commands can be used to install the dependencies & run the build tasks:
 
 ``` bash
 # Install NPM Dependencies
@@ -113,4 +113,4 @@ docker-compose run app php vendor/bin/phpunit
 ### Debugging
 
 The docker-compose setup ships with Xdebug, which you can listen to on port 9090.
-NB: For some editors like Visual Studio Code, you might need to map your workspace folder to the /app folder within the docker container for this to work.
+NB : For some editors like Visual Studio Code, you might need to map your workspace folder to the /app folder within the docker container for this to work.

dev/docs/php-testing.md

@@ -4,7 +4,7 @@ BookStack has many test cases defined within the `tests/` directory of the app.
 
 ## Setup
 
-The application tests are mostly functional, rather than unit tests, meaning they simulate user actions and system components, and therefore these require use of the database. To avoid potential conflicts within your development environment, the tests use a separate database. This is defined via a specific `mysql_testing` database connection in our configuration, and expects to use the following database access details:
+The application tests are mostly functional, rather than unit tests, meaning they simulate user actions and system components and therefore these require use of the database. To avoid potential conflicts within your development environment, the tests use a separate database. This is defined via a specific `mysql_testing` database connection in our configuration, and expects to use the following database access details:
 
 - Host: `127.0.0.1`
 - Username: `bookstack-test`

dev/licensing/php-library-licenses.txt

@@ -13,7 +13,7 @@ Link: http://aws.amazon.com/sdkforphp
 bacon/bacon-qr-code
 License: BSD-2-Clause
 License File: vendor/bacon/bacon-qr-code/LICENSE
-Copyright: Copyright (c) 2017-present, Ben Scholzen 'DASPRiD'
+Copyright: Copyright (c) 2017, Ben Scholzen 'DASPRiD'
 All rights reserved.
 Source: https://github.com/Bacon/BaconQrCode.git
 Link: https://github.com/Bacon/BaconQrCode

lang/sq/common.php

@@ -89,8 +89,8 @@ return [
     'homepage' => 'Homepage',
     'header_menu_expand' => 'Expand Header Menu',
     'profile_menu' => 'Profile Menu',
-    'view_profile' => 'Shiko profilin',
-    'edit_profile' => 'Ndrysho profilin',
+    'view_profile' => 'View Profile',
+    'edit_profile' => 'Edit Profile',
     'dark_mode' => 'Dark Mode',
     'light_mode' => 'Light Mode',
     'global_search' => 'Global Search',

tests/Activity/CommentsApiTest.php

@@ -1,6 +1,6 @@
 <?php
 
-namespace Tests\Activity;
+namespace Activity;
 
 use BookStack\Activity\Models\Comment;
 use BookStack\Permissions\Permission;

tests/Api/ApiDocsTest.php

@@ -22,7 +22,7 @@ class ApiDocsTest extends TestCase
         $resp->assertStatus(200);
         $resp->assertSee(url('/api/docs.json'));
         $resp->assertSee('Show a JSON view of the API docs data.');
-        $resp->assertHeader('Content-Type', 'text/html; charset=utf-8');
+        $resp->assertHeader('Content-Type', 'text/html; charset=UTF-8');
     }
 
     public function test_docs_json_endpoint_returns_json()

tests/Auth/OidcTest.php

@@ -138,7 +138,7 @@ class OidcTest extends TestCase
     {
         // Start auth
         $this->post('/oidc/login');
-        $state = explode(':', session()->get('oidc_state'), 2)[1];
+        $state = session()->get('oidc_state');
 
         $transactions = $this->mockHttpClient([$this->getMockAuthorizationResponse([
             'email' => 'benny@example.com',
@@ -190,35 +190,6 @@ class OidcTest extends TestCase
         $this->assertSessionError('Login using SingleSignOn-Testing failed, system did not provide successful authorization');
     }
 
-    public function test_callback_works_even_if_other_request_made_by_session()
-    {
-        $this->mockHttpClient([$this->getMockAuthorizationResponse([
-            'email' => 'benny@example.com',
-            'sub'   => 'benny1010101',
-        ])]);
-
-        $this->post('/oidc/login');
-        $state = explode(':', session()->get('oidc_state'), 2)[1];
-
-        $this->get('/');
-
-        $resp = $this->get("/oidc/callback?code=SplxlOBeZQQYbYS6WxSbIA&state={$state}");
-        $resp->assertRedirect('/');
-    }
-
-    public function test_callback_fails_if_state_timestamp_is_too_old()
-    {
-        $this->post('/oidc/login');
-        $state = explode(':', session()->get('oidc_state'), 2)[1];
-        session()->put('oidc_state', (time() - 60 * 4) . ':' . $state);
-
-        $this->get('/');
-
-        $resp = $this->get("/oidc/callback?code=SplxlOBeZQQYbYS6WxSbIA&state={$state}");
-        $resp->assertRedirect('/login');
-        $this->assertSessionError('Login using SingleSignOn-Testing failed, system did not provide successful authorization');
-    }
-
     public function test_dump_user_details_option_outputs_as_expected()
     {
         config()->set('oidc.dump_user_details', true);
@@ -826,7 +797,7 @@ class OidcTest extends TestCase
     {
         // Start auth
         $resp = $this->post('/oidc/login');
-        $state = explode(':', session()->get('oidc_state'), 2)[1];
+        $state = session()->get('oidc_state');
 
         $pkceCode = session()->get('oidc_pkce_code');
         $this->assertGreaterThan(30, strlen($pkceCode));
@@ -854,7 +825,7 @@ class OidcTest extends TestCase
     {
         config()->set('oidc.display_name_claims', 'first_name|last_name');
         $this->post('/oidc/login');
-        $state = explode(':', session()->get('oidc_state'), 2)[1];
+        $state = session()->get('oidc_state');
 
         $client = $this->mockHttpClient([
             $this->getMockAuthorizationResponse(['name' => null]),
@@ -1002,7 +973,7 @@ class OidcTest extends TestCase
         ]);
 
         $this->post('/oidc/login');
-        $state = explode(':', session()->get('oidc_state'), 2)[1];
+        $state = session()->get('oidc_state');
         $client = $this->mockHttpClient([$this->getMockAuthorizationResponse([
             'groups' => [],
         ])]);
@@ -1028,7 +999,7 @@ class OidcTest extends TestCase
     protected function runLogin($claimOverrides = [], $additionalHttpResponses = []): TestResponse
     {
         $this->post('/oidc/login');
-        $state = explode(':', session()->get('oidc_state'), 2)[1] ?? '';
+        $state = session()->get('oidc_state');
         $this->mockHttpClient([$this->getMockAuthorizationResponse($claimOverrides), ...$additionalHttpResponses]);
 
         return $this->get('/oidc/callback?code=SplxlOBeZQQYbYS6WxSbIA&state=' . $state);

tests/Auth/Saml2Test.php

@@ -36,7 +36,7 @@ class Saml2Test extends TestCase
     public function test_metadata_endpoint_displays_xml_as_expected()
     {
         $req = $this->get('/saml2/metadata');
-        $req->assertHeader('Content-Type', 'text/xml; charset=utf-8');
+        $req->assertHeader('Content-Type', 'text/xml; charset=UTF-8');
         $req->assertSee('md:EntityDescriptor');
         $req->assertSee(url('/saml2/acs'));
     }
@@ -51,7 +51,7 @@ class Saml2Test extends TestCase
 
         $req = $this->get('/saml2/metadata');
         $req->assertOk();
-        $req->assertHeader('Content-Type', 'text/xml; charset=utf-8');
+        $req->assertHeader('Content-Type', 'text/xml; charset=UTF-8');
         $req->assertSee('md:EntityDescriptor');
     }
 

tests/Entity/EntityQueryTest.php

@@ -1,44 +0,0 @@
-<?php
-
-namespace Tests\Entity;
-
-use BookStack\Entities\Models\Book;
-use Illuminate\Database\Eloquent\Builder;
-use Tests\TestCase;
-
-class EntityQueryTest extends TestCase
-{
-    public function test_basic_entity_query_has_join_and_type_applied()
-    {
-        $query = Book::query();
-        $expected = 'select * from `entities` left join `entity_container_data` on `entity_container_data`.`entity_id` = `entities`.`id` and `entity_container_data`.`entity_type` = ? where `type` = ? and `entities`.`deleted_at` is null';
-        $this->assertEquals($expected, $query->toSql());
-        $this->assertEquals(['book', 'book'], $query->getBindings());
-    }
-
-    public function test_joins_in_sub_queries_use_alias_names()
-    {
-        $query = Book::query()->whereHas('chapters', function (Builder $query) {
-            $query->where('name', '=', 'a');
-        });
-
-        // Probably from type limits on relation where not needed?
-        $expected = 'select * from `entities` left join `entity_container_data` on `entity_container_data`.`entity_id` = `entities`.`id` and `entity_container_data`.`entity_type` = ? where exists (select * from `entities` as `laravel_reserved_%d` left join `entity_container_data` on `entity_container_data`.`entity_id` = `laravel_reserved_%d`.`id` and `entity_container_data`.`entity_type` = ? where `entities`.`id` = `laravel_reserved_%d`.`book_id` and `name` = ? and `type` = ? and `laravel_reserved_%d`.`deleted_at` is null) and `type` = ? and `entities`.`deleted_at` is null';
-        $this->assertStringMatchesFormat($expected, $query->toSql());
-        $this->assertEquals(['book', 'chapter', 'a', 'chapter', 'book'], $query->getBindings());
-    }
-
-    public function test_book_chapter_relation_applies_type_condition()
-    {
-        $book = $this->entities->book();
-        $query = $book->chapters();
-        $expected = 'select * from `entities` left join `entity_container_data` on `entity_container_data`.`entity_id` = `entities`.`id` and `entity_container_data`.`entity_type` = ? where `entities`.`book_id` = ? and `entities`.`book_id` is not null and `type` = ? and `entities`.`deleted_at` is null';
-        $this->assertEquals($expected, $query->toSql());
-        $this->assertEquals(['chapter', $book->id, 'chapter'], $query->getBindings());
-
-        $query = Book::query()->whereHas('chapters');
-        $expected = 'select * from `entities` left join `entity_container_data` on `entity_container_data`.`entity_id` = `entities`.`id` and `entity_container_data`.`entity_type` = ? where exists (select * from `entities` as `laravel_reserved_%d` left join `entity_container_data` on `entity_container_data`.`entity_id` = `laravel_reserved_%d`.`id` and `entity_container_data`.`entity_type` = ? where `entities`.`id` = `laravel_reserved_%d`.`book_id` and `type` = ? and `laravel_reserved_%d`.`deleted_at` is null) and `type` = ? and `entities`.`deleted_at` is null';
-        $this->assertStringMatchesFormat($expected, $query->toSql());
-        $this->assertEquals(['book', 'chapter', 'chapter', 'book'], $query->getBindings());
-    }
-}

tests/Exports/ZipExportTest.php

@@ -374,54 +374,6 @@ class ZipExportTest extends TestCase
         $this->assertStringContainsString("<a href=\"{$ref}\">Original URL</a><a href=\"{$ref}\">Storage URL</a>", $pageData['html']);
     }
 
-    public function test_orphaned_images_can_be_used_on_default_local_storage()
-    {
-        $this->asEditor();
-        $page = $this->entities->page();
-        $result = $this->files->uploadGalleryImageToPage($this, $page);
-        $displayThumb = $result['response']->thumbs->gallery ?? '';
-        $page->html = '<p><img src="' . $displayThumb . '" alt="My image"></p>';
-        $page->save();
-
-        $image = Image::findOrFail($result['response']->id);
-        $image->uploaded_to = null;
-        $image->save();
-
-        $zipResp = $this->asEditor()->get($page->getUrl("/export/zip"));
-        $zipResp->assertOk();
-        $zip = ZipTestHelper::extractFromZipResponse($zipResp);
-        $pageData = $zip->data['page'];
-
-        $this->assertCount(1, $pageData['images']);
-        $imageData = $pageData['images'][0];
-        $this->assertEquals($image->id, $imageData['id']);
-
-        $this->assertEquals('<p><img src="[[bsexport:image:' . $imageData['id'] . ']]" alt="My image"></p>', $pageData['html']);
-    }
-
-    public function test_orphaned_images_cannot_be_used_on_local_secure_restricted()
-    {
-        config()->set('filesystems.images', 'local_secure_restricted');
-
-        $this->asEditor();
-        $page = $this->entities->page();
-        $result = $this->files->uploadGalleryImageToPage($this, $page);
-        $displayThumb = $result['response']->thumbs->gallery ?? '';
-        $page->html = '<p><img src="' . $displayThumb . '" alt="My image"></p>';
-        $page->save();
-
-        $image = Image::findOrFail($result['response']->id);
-        $image->uploaded_to = null;
-        $image->save();
-
-        $zipResp = $this->asEditor()->get($page->getUrl("/export/zip"));
-        $zipResp->assertOk();
-        $zip = ZipTestHelper::extractFromZipResponse($zipResp);
-        $pageData = $zip->data['page'];
-
-        $this->assertCount(0, $pageData['images']);
-    }
-
     public function test_cross_reference_links_external_to_export_are_not_converted()
     {
         $page = $this->entities->page();

tests/SessionTest.php

@@ -1,53 +0,0 @@
-<?php
-
-namespace Tests;
-
-class SessionTest extends TestCase
-{
-    public function test_secure_images_not_tracked_in_session_history()
-    {
-        config()->set('filesystems.images', 'local_secure');
-        $this->asEditor();
-        $page = $this->entities->page();
-        $result = $this->files->uploadGalleryImageToPage($this, $page);
-        $expectedPath = storage_path($result['path']);
-        $this->assertFileExists($expectedPath);
-
-        $this->get('/books');
-        $this->assertEquals(url('/books'), session()->previousUrl());
-
-        $resp = $this->get($result['path']);
-        $resp->assertOk();
-        $resp->assertHeader('Content-Type', 'image/png');
-
-        $this->assertEquals(url('/books'), session()->previousUrl());
-
-        if (file_exists($expectedPath)) {
-            unlink($expectedPath);
-        }
-    }
-
-    public function test_pwa_manifest_is_not_tracked_in_session_history()
-    {
-        $this->asEditor()->get('/books');
-        $this->get('/manifest.json');
-
-        $this->assertEquals(url('/books'), session()->previousUrl());
-    }
-
-    public function test_dist_dir_access_is_not_tracked_in_session_history()
-    {
-        $this->asEditor()->get('/books');
-        $this->get('/dist/sub/hello.txt');
-
-        $this->assertEquals(url('/books'), session()->previousUrl());
-    }
-
-    public function test_opensearch_is_not_tracked_in_session_history()
-    {
-        $this->asEditor()->get('/books');
-        $this->get('/opensearch.xml');
-
-        $this->assertEquals(url('/books'), session()->previousUrl());
-    }
-}

tests/ThemeTest.php

@@ -478,7 +478,7 @@ END;
 
             $resp = $this->asAdmin()->get("/theme/{$themeFolderName}/file.txt");
             $resp->assertStreamedContent($text);
-            $resp->assertHeader('Content-Type', 'text/plain; charset=utf-8');
+            $resp->assertHeader('Content-Type', 'text/plain; charset=UTF-8');
             $resp->assertHeader('Cache-Control', 'max-age=86400, private');
 
             $resp = $this->asAdmin()->get("/theme/{$themeFolderName}/image.png");
@@ -487,7 +487,7 @@ END;
 
             $resp = $this->asAdmin()->get("/theme/{$themeFolderName}/file.css");
             $resp->assertStreamedContent($css);
-            $resp->assertHeader('Content-Type', 'text/css; charset=utf-8');
+            $resp->assertHeader('Content-Type', 'text/css; charset=UTF-8');
             $resp->assertHeader('Cache-Control', 'max-age=86400, private');
         });
     }

tests/Uploads/AttachmentTest.php

@@ -323,7 +323,7 @@ class AttachmentTest extends TestCase
 
         $attachmentGet = $this->get($attachment->getUrl(true));
         // http-foundation/Response does some 'fixing' of responses to add charsets to text responses.
-        $attachmentGet->assertHeader('Content-Type', 'text/plain; charset=utf-8');
+        $attachmentGet->assertHeader('Content-Type', 'text/plain; charset=UTF-8');
         $attachmentGet->assertHeader('Content-Disposition', 'inline; filename="upload_test_file.txt"');
         $attachmentGet->assertHeader('X-Content-Type-Options', 'nosniff');
 
@@ -339,7 +339,7 @@ class AttachmentTest extends TestCase
 
         $attachmentGet = $this->get($attachment->getUrl(true));
         // http-foundation/Response does some 'fixing' of responses to add charsets to text responses.
-        $attachmentGet->assertHeader('Content-Type', 'text/plain; charset=utf-8');
+        $attachmentGet->assertHeader('Content-Type', 'text/plain; charset=UTF-8');
         $attachmentGet->assertHeader('Content-Disposition', 'inline; filename="test_file.html"');
 
         $this->files->deleteAllAttachmentFiles();

tests/Uploads/ImageTest.php

@@ -5,8 +5,6 @@ namespace Tests\Uploads;
 use BookStack\Entities\Repos\PageRepo;
 use BookStack\Uploads\Image;
 use BookStack\Uploads\ImageService;
-use BookStack\Uploads\UserAvatars;
-use BookStack\Users\Models\Role;
 use Illuminate\Support\Str;
 use Tests\TestCase;
 
@@ -75,10 +73,6 @@ class ImageTest extends TestCase
 
     public function test_image_display_thumbnail_generation_for_animated_avif_images_uses_original_file()
     {
-        if (! function_exists('imageavif')) {
-            $this->markTestSkipped('imageavif() is not available');
-        }
-
         $page = $this->entities->page();
         $admin = $this->users->admin();
         $this->actingAs($admin);
@@ -429,6 +423,29 @@ class ImageTest extends TestCase
         }
     }
 
+    public function test_secure_images_not_tracked_in_session_history()
+    {
+        config()->set('filesystems.images', 'local_secure');
+        $this->asEditor();
+        $page = $this->entities->page();
+        $result = $this->files->uploadGalleryImageToPage($this, $page);
+        $expectedPath = storage_path($result['path']);
+        $this->assertFileExists($expectedPath);
+
+        $this->get('/books');
+        $this->assertEquals(url('/books'), session()->previousUrl());
+
+        $resp = $this->get($result['path']);
+        $resp->assertOk();
+        $resp->assertHeader('Content-Type', 'image/png');
+
+        $this->assertEquals(url('/books'), session()->previousUrl());
+
+        if (file_exists($expectedPath)) {
+            unlink($expectedPath);
+        }
+    }
+
     public function test_system_images_remain_public_with_local_secure_restricted()
     {
         config()->set('filesystems.images', 'local_secure_restricted');
@@ -446,26 +463,6 @@ class ImageTest extends TestCase
         }
     }
 
-    public function test_avatar_images_visible_only_when_public_access_enabled_with_local_secure_restricted()
-    {
-        config()->set('filesystems.images', 'local_secure_restricted');
-        $user = $this->users->admin();
-        $avatars = $this->app->make(UserAvatars::class);
-        $avatars->assignToUserFromExistingData($user, $this->files->pngImageData(), 'png');
-
-        $avatarUrl = $user->getAvatar();
-
-        $resp = $this->get($avatarUrl);
-        $resp->assertRedirect('/login');
-
-        $this->permissions->makeAppPublic();
-
-        $resp = $this->get($avatarUrl);
-        $resp->assertOk();
-
-        $this->files->deleteAtRelativePath($user->avatar->path);
-    }
-
     public function test_secure_restricted_images_inaccessible_without_relation_permission()
     {
         config()->set('filesystems.images', 'local_secure_restricted');
@@ -490,38 +487,6 @@ class ImageTest extends TestCase
         }
     }
 
-    public function test_secure_restricted_images_accessible_with_public_guest_access()
-    {
-        config()->set('filesystems.images', 'local_secure_restricted');
-        $this->permissions->makeAppPublic();
-
-        $this->asEditor();
-        $page = $this->entities->page();
-        $this->files->uploadGalleryImageToPage($this, $page);
-        $image = Image::query()->where('type', '=', 'gallery')
-            ->where('uploaded_to', '=', $page->id)
-            ->first();
-
-        $expectedUrl = url($image->path);
-        $expectedPath = storage_path($image->path);
-        auth()->logout();
-
-        $this->get($expectedUrl)->assertOk();
-
-        $this->permissions->setEntityPermissions($page, [], []);
-
-        $resp = $this->get($expectedUrl);
-        $resp->assertNotFound();
-
-        $this->permissions->setEntityPermissions($page, ['view'], [Role::getSystemRole('public')]);
-
-        $this->get($expectedUrl)->assertOk();
-
-        if (file_exists($expectedPath)) {
-            unlink($expectedPath);
-        }
-    }
-
     public function test_thumbnail_path_handled_by_secure_restricted_images()
     {
         config()->set('filesystems.images', 'local_secure_restricted');

version

@@ -1 +1 @@
-v25.11.5
+v25.02-dev