improved error handling and tests added

.forgejo/FUNDING.yml

@@ -1,4 +0,0 @@
-# These are supported funding model platforms
-
-github: [ssddanbrown]
-ko_fi: ssddanbrown

.github/workflows/analyse-php.yml

@@ -1,7 +1,6 @@
 name: analyse-php
 
 on:
-  workflow_dispatch:
   push:
     paths:
       - '**.php'
@@ -12,16 +11,14 @@ on:
 jobs:
   build:
     if: ${{ github.ref != 'refs/heads/l10n_development' }}
-    runs-on: docker
+    runs-on: ubuntu-24.04
-    container:
-      image: node:24-bullseye
     steps:
-    - uses: https://code.forgejo.org/actions/checkout@v6
+    - uses: actions/checkout@v4
 
     - name: Setup PHP
-      uses: https://github.com/shivammathur/setup-php@v2
+      uses: shivammathur/setup-php@v2
       with:
-        php-version: 8.5
+        php-version: 8.3
         extensions: gd, mbstring, json, curl, xml, mysql, ldap
 
     - name: Get Composer Cache Directory
@@ -30,16 +27,14 @@ jobs:
         echo "dir=$(composer config cache-files-dir)" >> $GITHUB_OUTPUT
 
     - name: Cache composer packages
-      uses: https://code.forgejo.org/actions/cache@v5
+      uses: actions/cache@v4
       with:
         path: ${{ steps.composer-cache.outputs.dir }}
-        key: ${{ runner.os }}-composer-8.5
+        key: ${{ runner.os }}-composer-8.3
         restore-keys: ${{ runner.os }}-composer-
 
     - name: Install composer dependencies
       run: composer install --prefer-dist --no-interaction --ansi
-      env:
-        COMPOSER_AUTH: '{"github-oauth": {"github.com": "${{ secrets.GH_TOKEN }}"}}'
 
     - name: Run static analysis check
       run: composer check-static

.github/workflows/lint-js.yml

@@ -1,7 +1,6 @@
 name: lint-js
 
 on:
-  workflow_dispatch:
   push:
     paths:
       - '**.js'
@@ -14,11 +13,9 @@ on:
 jobs:
   build:
     if: ${{ github.ref != 'refs/heads/l10n_development' }}
-    runs-on: docker
+    runs-on: ubuntu-24.04
-    container:
-      image: node:24-bullseye
     steps:
-    - uses: https://code.forgejo.org/actions/checkout@v6
+    - uses: actions/checkout@v4
 
     - name: Install NPM deps
       run: npm ci

.github/workflows/lint-php.yml

@@ -1,7 +1,6 @@
 name: lint-php
 
 on:
-  workflow_dispatch:
   push:
     paths:
       - '**.php'
@@ -12,16 +11,14 @@ on:
 jobs:
   build:
     if: ${{ github.ref != 'refs/heads/l10n_development' }}
-    runs-on: docker
+    runs-on: ubuntu-24.04
-    container:
-      image: node:24-bullseye
     steps:
-    - uses: https://code.forgejo.org/actions/checkout@v6
+    - uses: actions/checkout@v4
 
     - name: Setup PHP
-      uses: https://github.com/shivammathur/setup-php@v2
+      uses: shivammathur/setup-php@v2
       with:
-        php-version: 8.5
+        php-version: 8.3
         tools: phpcs
 
     - name: Run formatting check

.github/workflows/test-js.yml

@@ -1,7 +1,6 @@
 name: test-js
 
 on:
-  workflow_dispatch:
   push:
     paths:
       - '**.js'
@@ -16,11 +15,9 @@ on:
 jobs:
   build:
     if: ${{ github.ref != 'refs/heads/l10n_development' }}
-    runs-on: docker
+    runs-on: ubuntu-24.04
-    container:
-      image: node:24-bullseye
     steps:
-    - uses: https://code.forgejo.org/actions/checkout@v6
+    - uses: actions/checkout@v6
 
     - name: Install NPM deps
       run: npm ci

.github/workflows/test-migrations.yml

@@ -1,7 +1,6 @@
 name: test-migrations
 
 on:
-  workflow_dispatch:
   push:
     paths:
       - '**.php'
@@ -14,25 +13,15 @@ on:
 jobs:
   build:
     if: ${{ github.ref != 'refs/heads/l10n_development' }}
-    runs-on: docker
+    runs-on: ubuntu-24.04
-    container:
-      image: node:24-bullseye
     strategy:
       matrix:
         php: ['8.2', '8.3', '8.4', '8.5']
-    services:
-      mysql:
-        image: docker.io/library/mariadb:12.2.2-noble
-        env:
-          MARIADB_USER: bookstack-test
-          MARIADB_PASSWORD: bookstack-test
-          MARIADB_DATABASE: bookstack-test
-          MARIADB_ROOT_PASSWORD: password
     steps:
-      - uses: https://code.forgejo.org/actions/checkout@v6
+      - uses: actions/checkout@v4
 
       - name: Setup PHP
-        uses: https://github.com/shivammathur/setup-php@v2
+        uses: shivammathur/setup-php@v2
         with:
           php-version: ${{ matrix.php }}
           extensions: gd, mbstring, json, curl, xml, mysql, ldap
@@ -43,31 +32,34 @@ jobs:
           echo "dir=$(composer config cache-files-dir)" >> $GITHUB_OUTPUT
 
       - name: Cache composer packages
-        uses: https://code.forgejo.org/actions/cache@v5
+        uses: actions/cache@v4
         with:
           path: ${{ steps.composer-cache.outputs.dir }}
           key: ${{ runner.os }}-composer-${{ matrix.php }}
           restore-keys: ${{ runner.os }}-composer-
 
+      - name: Start MySQL
+        run: |
+          sudo systemctl start mysql
+
+      - name: Create database & user
+        run: |
+          mysql -uroot -proot -e 'CREATE DATABASE IF NOT EXISTS `bookstack-test`;'
+          mysql -uroot -proot -e "CREATE USER 'bookstack-test'@'localhost' IDENTIFIED WITH mysql_native_password BY 'bookstack-test';"
+          mysql -uroot -proot -e "GRANT ALL ON \`bookstack-test\`.* TO 'bookstack-test'@'localhost';"
+          mysql -uroot -proot -e 'FLUSH PRIVILEGES;'
+
       - name: Install composer dependencies
         run: composer install --prefer-dist --no-interaction --ansi
-        env:
-          COMPOSER_AUTH: '{"github-oauth": {"github.com": "${{ secrets.GH_TOKEN }}"}}'
 
       - name: Start migration test
-        env:
-          DB_HOST: mysql
         run: |
           php${{ matrix.php }} artisan migrate --force -n --database=mysql_testing
 
       - name: Start migration:rollback test
-        env:
-          DB_HOST: mysql
         run: |
           php${{ matrix.php }} artisan migrate:rollback --force -n --database=mysql_testing
 
       - name: Start migration rerun test
-        env:
-          DB_HOST: mysql
         run: |
           php${{ matrix.php }} artisan migrate --force -n --database=mysql_testing

.github/workflows/test-php.yml

@@ -1,7 +1,6 @@
 name: test-php
 
 on:
-  workflow_dispatch:
   push:
     paths:
       - '**.php'
@@ -14,25 +13,15 @@ on:
 jobs:
   build:
     if: ${{ github.ref != 'refs/heads/l10n_development' }}
-    runs-on: docker
+    runs-on: ubuntu-24.04
-    container:
-      image: node:24-bullseye
     strategy:
       matrix:
         php: ['8.2', '8.3', '8.4', '8.5']
-    services:
-      mysql:
-        image: docker.io/library/mariadb:12.2.2-noble
-        env:
-          MARIADB_USER: bookstack-test
-          MARIADB_PASSWORD: bookstack-test
-          MARIADB_DATABASE: bookstack-test
-          MARIADB_ROOT_PASSWORD: password
     steps:
-    - uses: https://code.forgejo.org/actions/checkout@v6
+    - uses: actions/checkout@v4
 
     - name: Setup PHP
-      uses: https://github.com/shivammathur/setup-php@v2
+      uses: shivammathur/setup-php@v2
       with:
         php-version: ${{ matrix.php }}
         extensions: gd, mbstring, json, curl, xml, mysql, ldap, gmp
@@ -43,25 +32,30 @@ jobs:
         echo "dir=$(composer config cache-files-dir)" >> $GITHUB_OUTPUT
 
     - name: Cache composer packages
-      uses: https://code.forgejo.org/actions/cache@v5
+      uses: actions/cache@v4
       with:
         path: ${{ steps.composer-cache.outputs.dir }}
         key: ${{ runner.os }}-composer-${{ matrix.php }}
         restore-keys: ${{ runner.os }}-composer-
 
+    - name: Start Database
+      run: |
+        sudo systemctl start mysql
+
+    - name: Setup Database
+      run: |
+        mysql -uroot -proot -e 'CREATE DATABASE IF NOT EXISTS `bookstack-test`;'
+        mysql -uroot -proot -e "CREATE USER 'bookstack-test'@'localhost' IDENTIFIED WITH mysql_native_password BY 'bookstack-test';"
+        mysql -uroot -proot -e "GRANT ALL ON \`bookstack-test\`.* TO 'bookstack-test'@'localhost';"
+        mysql -uroot -proot -e 'FLUSH PRIVILEGES;'
+
     - name: Install composer dependencies
       run: composer install --prefer-dist --no-interaction --ansi
-      env:
-        COMPOSER_AUTH: '{"github-oauth": {"github.com": "${{ secrets.GH_TOKEN }}"}}'
 
     - name: Migrate and seed the database
-      env:
-        DB_HOST: mysql
       run: |
         php${{ matrix.php }} artisan migrate --force -n --database=mysql_testing
         php${{ matrix.php }} artisan db:seed --force -n --class=DummyContentSeeder --database=mysql_testing
 
     - name: Run PHP tests
-      env:
-        DB_HOST: mysql
       run: php${{ matrix.php }} ./vendor/bin/phpunit

app/Access/Controllers/OidcController.php

@@ -55,6 +55,7 @@ class OidcController extends Controller
         }
 
         try {
+            $this->throwIfAuthorizationError($request);
             $this->oidcService->processAuthorizeResponse($request->query('code'));
         } catch (OidcException $oidcException) {
             $this->showErrorNotification($oidcException->getMessage());
@@ -72,4 +73,23 @@ class OidcController extends Controller
     {
         return redirect($this->oidcService->logout());
     }
+
+    /**
+     *
+     * @throws OidcException
+     */
+    private function throwIfAuthorizationError(Request $request): void
+    {
+        $errorCode = $request->query('error');
+        if (!$errorCode) {
+            return;
+        }
+
+        $errorDescription = $request->query('error_description');
+        if ($errorDescription) {
+            throw new OidcException($errorDescription);
+        }
+
+        throw new OidcException(trans('errors.oidc_fail_authed', ['system' => config('oidc.name')]));
+    }
 }

app/Access/Oidc/OidcHttpBasicWithClientIdOptionProvider.php

@@ -0,0 +1,27 @@
+<?php
+
+namespace BookStack\Access\Oidc;
+
+use League\OAuth2\Client\OptionProvider\HttpBasicAuthOptionProvider;
+
+/**
+ * Option provider that sends credentials via HTTP Basic Auth header
+ * and also includes client_id in the request body.
+ */
+class OidcHttpBasicWithClientIdOptionProvider extends HttpBasicAuthOptionProvider
+{
+    public function getAccessTokenOptions($method, array $params)
+    {
+        $clientId = $params['client_id'] ?? null;
+
+        $options = parent::getAccessTokenOptions($method, $params);
+
+        if ($clientId) {
+            parse_str($options['body'] ?? '', $body);
+            $body['client_id'] = $clientId;
+            $options['body'] = http_build_query($body);
+        }
+
+        return $options;
+    }
+}

app/Access/Oidc/OidcIdToken.php

@@ -9,10 +9,10 @@ class OidcIdToken extends OidcJwtWithClaims implements ProvidesClaims
      *
      * @throws OidcInvalidTokenException
      */
-    public function validate(string $clientId): bool
+    public function validate(OidcProviderSettings $settings): bool
     {
-        parent::validateCommonTokenDetails($clientId);
+        parent::validateCommonTokenDetails($settings);
-        $this->validateTokenClaims($clientId);
+        $this->validateTokenClaims($settings->clientId);
 
         return true;
     }

app/Access/Oidc/OidcJwtWithClaims.php

@@ -9,7 +9,9 @@ class OidcJwtWithClaims implements ProvidesClaims
     protected string $signature;
     protected string $issuer;
     protected array $tokenParts = [];
-
+    protected array $acceptedSignatures = [self::hs256Signature, self::rs256Signature];
+    private const hs256Signature = 'HS256'
+        , rs256Signature = 'RS256';
     /**
      * @var array[]|string[]
      */
@@ -59,11 +61,11 @@ class OidcJwtWithClaims implements ProvidesClaims
      *
      * @throws OidcInvalidTokenException
      */
-    public function validateCommonTokenDetails(string $clientId): bool
+    public function validateCommonTokenDetails(OidcProviderSettings $settings): bool
     {
         $this->validateTokenStructure();
-        $this->validateTokenSignature();
+        $this->validateTokenSignature($settings);
-        $this->validateCommonClaims($clientId);
+        $this->validateCommonClaims($settings->clientId);
 
         return true;
     }
@@ -102,12 +104,12 @@ class OidcJwtWithClaims implements ProvidesClaims
     protected function validateTokenStructure(): void
     {
         foreach (['header', 'payload'] as $prop) {
-            if (empty($this->$prop)) {
+            if (empty($this->$prop) || !is_array($this->$prop)) {
                 throw new OidcInvalidTokenException("Could not parse out a valid {$prop} within the provided token");
             }
         }
 
-        if (empty($this->signature)) {
+        if (empty($this->signature) || !is_string($this->signature)) {
             throw new OidcInvalidTokenException('Could not parse out a valid signature within the provided token');
         }
     }
@@ -117,31 +119,42 @@ class OidcJwtWithClaims implements ProvidesClaims
      *
      * @throws OidcInvalidTokenException
      */
-    protected function validateTokenSignature(): void
+    protected function validateTokenSignature(OidcProviderSettings $settings): void {  
-    {
+        $validSignatures = implode(', ',$this->acceptedSignatures);
-        if ($this->header['alg'] !== 'RS256') {
+        switch ($this->header['alg']) {
-            throw new OidcInvalidTokenException("Only RS256 signature validation is supported. Token reports using {$this->header['alg']}");
+            case self::rs256Signature:
+                $parsedKeys = array_map(function ($key) {
+                    try {
+                        return new OidcJwtSigningKey($key);
+                    } catch (OidcInvalidKeyException $e) {
+                        throw new OidcInvalidTokenException('Failed to read signing key with error: ' . $e->getMessage());
+                    }
+                }, $this->keys);
+
+                $parsedKeys = array_filter($parsedKeys);
+
+                $contentToSign = $this->tokenParts[0] . '.' . $this->tokenParts[1];
+                /** @var OidcJwtSigningKey $parsedKey */
+                foreach ($parsedKeys as $parsedKey) {
+                    if ($parsedKey->verify($contentToSign, $this->signature)) {
+                        return;
+                    }
+                }
+            
+                throw new OidcInvalidTokenException('Token signature could not be validated using the provided keys');                
+            case self::hs256Signature:
+                $secret = $settings->clientSecret;
+                $contentToSign = $this->tokenParts[0] . '.' . $this->tokenParts[1];
+                $expectedSignature = hash_hmac('sha256', $contentToSign, $secret, true);
+
+                if (hash_equals($expectedSignature, $this->signature)) {
+                    return;
+                }
+            
+                throw new OidcInvalidTokenException('Token signature could not be validated using the provided secret');                
+            default:
+                throw new OidcInvalidTokenException("Only $validSignatures signatures validation are supported. Token reports using {$this->header['alg']}");
         }
-
-        $parsedKeys = array_map(function ($key) {
-            try {
-                return new OidcJwtSigningKey($key);
-            } catch (OidcInvalidKeyException $e) {
-                throw new OidcInvalidTokenException('Failed to read signing key with error: ' . $e->getMessage());
-            }
-        }, $this->keys);
-
-        $parsedKeys = array_filter($parsedKeys);
-
-        $contentToSign = $this->tokenParts[0] . '.' . $this->tokenParts[1];
-        /** @var OidcJwtSigningKey $parsedKey */
-        foreach ($parsedKeys as $parsedKey) {
-            if ($parsedKey->verify($contentToSign, $this->signature)) {
-                return;
-            }
-        }
-
-        throw new OidcInvalidTokenException('Token signature could not be validated using the provided keys');
     }
 
     /**

app/Access/Oidc/OidcProviderSettings.php

@@ -74,7 +74,7 @@ class OidcProviderSettings
     {
         $this->validateInitial();
 
-        $required = ['keys', 'tokenEndpoint', 'authorizationEndpoint'];
+        $required = ['tokenEndpoint', 'authorizationEndpoint'];
         foreach ($required as $prop) {
             if (empty($this->$prop)) {
                 throw new InvalidArgumentException("Missing required configuration \"{$prop}\" value");

app/Access/Oidc/OidcService.php

@@ -14,7 +14,6 @@ use BookStack\Theming\ThemeEvents;
 use BookStack\Uploads\UserAvatars;
 use BookStack\Users\Models\User;
 use Illuminate\Support\Facades\Cache;
-use League\OAuth2\Client\OptionProvider\HttpBasicAuthOptionProvider;
 use League\OAuth2\Client\Provider\Exception\IdentityProviderException;
 
 /**
@@ -140,7 +139,7 @@ class OidcService
             'redirectUri' => url('/oidc/callback'),
         ], [
             'httpClient'     => $this->http->buildClient(5),
-            'optionProvider' => new HttpBasicAuthOptionProvider(),
+            'optionProvider' => new OidcHttpBasicWithClientIdOptionProvider(),
         ]);
 
         foreach ($this->getAdditionalScopes() as $scope) {
@@ -199,7 +198,7 @@ class OidcService
         }
 
         try {
-            $idToken->validate($settings->clientId);
+            $idToken->validate($settings);
         } catch (OidcInvalidTokenException $exception) {
             throw new OidcException("ID token validation failed with error: {$exception->getMessage()}");
         }

app/Config/exports.php

@@ -68,7 +68,7 @@ return [
          * Times-Roman, Times-Bold, Times-BoldItalic, Times-Italic,
          * Symbol, ZapfDingbats.
          */
-        'font_dir' => storage_path('fonts/dompdf'),  // advised by dompdf (https://github.com/dompdf/dompdf/pull/782)
+        'font_dir' => storage_path('fonts/'),  // advised by dompdf (https://github.com/dompdf/dompdf/pull/782)
 
         /**
          * The location of the DOMPDF font cache directory.
@@ -78,7 +78,7 @@ return [
          *
          * Note: This directory must exist and be writable by the webserver process.
          */
-        'font_cache' => storage_path('fonts/dompdf/cache'),
+        'font_cache' => storage_path('fonts/'),
 
         /**
          * The location of a temporary directory.

app/Exports/PdfGenerator.php

@@ -4,8 +4,6 @@ namespace BookStack\Exports;
 
 use BookStack\Exceptions\PdfExportException;
 use Dompdf\Dompdf;
-use FontLib\Font;
-use Illuminate\Support\Str;
 use Knp\Snappy\Pdf as SnappyPdf;
 use Symfony\Component\Process\Exception\ProcessTimedOutException;
 use Symfony\Component\Process\Process;
@@ -62,65 +60,12 @@ class PdfGenerator
         $domPdf = new Dompdf($options);
         $domPdf->setBasePath(base_path('public'));
 
-        $fontMetrics = $domPdf->getFontMetrics();
-        $userFontfamilies = $this->getUserDomPdfFontFamilies();
-        foreach ($userFontfamilies as $fontFamily => $fonts) {
-            try {
-                $fontMetrics->setFontFamily($fontFamily, $fonts);
-            } catch (\Exception $exception) {
-                $expectedPath = storage_path('fonts/dompdf');
-                throw new PdfExportException("Failed to create required font data in {$expectedPath}, Ensure all content in this location is writable by the web server");
-            }
-        }
-
         $domPdf->loadHTML($this->convertEntities($html));
         $domPdf->render();
 
         return (string) $domPdf->output();
     }
 
-    /**
-     * @return array<string, array<string, string>>
-     */
-    protected function getUserDomPdfFontFamilies(): array
-    {
-        $fontStore = storage_path('fonts/dompdf');
-        if (!is_dir($fontStore)) {
-            return [];
-        }
-
-        $fontFamilies = [];
-        $fontFiles = glob($fontStore . DIRECTORY_SEPARATOR . '*.ttf');
-        foreach ($fontFiles as $fontFile) {
-            $fontFileName = basename($fontFile, '.ttf');
-            $expectedUfm = $fontStore . DIRECTORY_SEPARATOR . $fontFileName . '.ufm';
-            if (!file_exists($expectedUfm)) {
-                $font = Font::load($fontFile);
-                $font->parse();
-                try {
-                    $font->saveAdobeFontMetrics($expectedUfm);
-                } catch (\Exception $exception) {
-                    throw new PdfExportException("Failed to create required font data at $expectedUfm, Ensure this location is writable by the web server");
-                }
-            }
-
-            $nameParts = explode('-', $fontFileName);
-            if (count($nameParts) === 1 || $nameParts[1] === 'Regular') {
-                $nameParts[1] = 'Normal';
-            }
-
-            $family = trim(strtolower(preg_replace('/([A-Z])/', ' $1', $nameParts[0])));
-            $variation = Str::snake($nameParts[1]);
-            if (!isset($fontFamilies[$family])) {
-                $fontFamilies[$family] = [];
-            }
-
-            $fontFamilies[$family][$variation] = $fontStore . DIRECTORY_SEPARATOR . $fontFileName;
-        }
-
-        return $fontFamilies;
-    }
-
     /**
      * @throws PdfExportException
      */

readme.md

@@ -72,7 +72,7 @@ Big thanks to these companies for supporting the project.
 <td align="center"><a href="https://www.stellarhosted.com/bookstack/" target="_blank">
     <img width="240" src="https://www.bookstackapp.com/images/sponsors/stellarhosted.png" alt="Stellar Hosted">
 </a></td>
-<td align="center" style="text-align: center"><a href="https://nws.netways.de" target="_blank">
+<td align="center" style="text-align: center"><a href="https://nws.netways.de/apps/bookstack/" target="_blank">
     <img width="240" src="https://www.bookstackapp.com/images/sponsors/netways.png" alt="NETWAYS Web Services">
 </a></td>
 </tr>

storage/fonts/.gitignore

@@ -1,6 +1,2 @@
-# Font cache files have once been stored directly in this folder
-# therefore its important the contents non-ignored by git
-# are chosen selectively
 *
 !.gitignore
-!dompdf/

storage/fonts/dompdf/.gitignore

@@ -1,3 +0,0 @@
-*
-!.gitignore
-!cache/

storage/fonts/dompdf/cache/.gitignore

@@ -1,2 +0,0 @@
-*
-!.gitignore

tests/Exports/PdfExportTest.php

@@ -79,39 +79,6 @@ class PdfExportTest extends TestCase
         $this->assertStringContainsString('<details open="open"', $pdfHtml);
     }
 
-    public function test_custom_fonts_loaded_for_dom_pdf_when_used()
-    {
-        // Set up custom font usage
-        $page = $this->entities->page()->forceFill([
-            'html'     => '<p><strong>Bold</strong>text</p>',
-        ]);
-        $page->save();
-        $this->setSettings([
-            'app-custom-head' => '<style>* { font-family: "meow words"}</style>'
-        ]);
-        $normalFont = $this->files->testFilePath('fonts/Cardiff.ttf');
-        $normalFontTarget = storage_path('fonts/dompdf/MeowWords.ttf');
-        $boldFont = $this->files->testFilePath('fonts/Cardiff-Bold.ttf');
-        $boldFontTarget = storage_path('fonts/dompdf/MeowWords-Bold.ttf');
-        copy($normalFont, $normalFontTarget);
-        copy($boldFont, $boldFontTarget);
-
-        $resp = $this->asEditor()->get($page->getUrl('/export/pdf'));
-        $resp->assertStatus(200);
-
-        // Existance of UFM files indicates the metrics have been generated
-        $this->assertFileExists(storage_path('fonts/dompdf/MeowWords.ufm'));
-        $this->assertFileExists(storage_path('fonts/dompdf/MeowWords-Bold.ufm'));
-        // Existence of cache json files indicates the fonts have been used
-        $this->assertFileExists(storage_path('fonts/dompdf/cache/MeowWords.ufm.json'));
-        $this->assertFileExists(storage_path('fonts/dompdf/cache/MeowWords-Bold.ufm.json'));
-
-        $filesToCleanUp = [...glob(storage_path('fonts/dompdf/Meow*')), ...glob(storage_path('fonts/dompdf/cache/Meow*'))];
-        foreach ($filesToCleanUp as $file) {
-            unlink($file);
-        }
-    }
-
     public function test_wkhtmltopdf_only_used_when_allow_untrusted_is_true()
     {
         $page = $this->entities->page();

tests/Helpers/OidcJwtHelper.php

@@ -2,6 +2,7 @@
 
 namespace Tests\Helpers;
 
+use BookStack\Access\Oidc\OidcProviderSettings;
 use phpseclib3\Crypt\RSA;
 
 /**
@@ -119,6 +120,41 @@ q/1PY4iJviGKddtmfClH3v4=
 -----END PRIVATE KEY-----';
     }
 
+    public static function defaultSecret(): string
+    {
+        return 'test-client-secret-for-hs256';
+    }
+
+    /**
+     * Build a minimal OidcProviderSettings for use in token validation tests.
+     */
+    public static function defaultProviderSettings(array $overrides = []): OidcProviderSettings
+    {
+        return new OidcProviderSettings(array_merge([
+            'issuer'       => static::defaultIssuer(),
+            'clientId'     => static::defaultClientId(),
+            'clientSecret' => static::defaultSecret(),
+        ], $overrides));
+    }
+
+    /**
+     * Build an HS256-signed ID token using the given secret.
+     */
+    public static function hs256IdToken(string $secret, array $payloadOverrides = []): string
+    {
+        $payload = array_merge(static::defaultPayload(), $payloadOverrides);
+        $header = ['alg' => 'HS256', 'typ' => 'JWT'];
+
+        $top = implode('.', [
+            static::base64UrlEncode(json_encode($header)),
+            static::base64UrlEncode(json_encode($payload)),
+        ]);
+
+        $signature = hash_hmac('sha256', $top, $secret, true);
+
+        return $top . '.' . static::base64UrlEncode($signature);
+    }
+
     public static function publicJwkKeyArray(): array
     {
         return [

tests/Unit/OidcIdTokenTest.php

@@ -15,7 +15,7 @@ class OidcIdTokenTest extends TestCase
             OidcJwtHelper::publicJwkKeyArray(),
         ]);
 
-        $this->assertTrue($token->validate('xxyyzz.aaa.bbccdd.123'));
+        $this->assertTrue($token->validate(OidcJwtHelper::defaultProviderSettings()));
     }
 
     public function test_get_claim_returns_value_if_existing()
@@ -56,7 +56,7 @@ class OidcIdTokenTest extends TestCase
             $err = null;
 
             try {
-                $token->validate('abc');
+                $token->validate(OidcJwtHelper::defaultProviderSettings());
             } catch (\Exception $exception) {
                 $err = $exception;
             }
@@ -71,7 +71,7 @@ class OidcIdTokenTest extends TestCase
         $token = new OidcIdToken(OidcJwtHelper::idToken(), OidcJwtHelper::defaultIssuer(), []);
         $this->expectException(OidcInvalidTokenException::class);
         $this->expectExceptionMessage('Token signature could not be validated using the provided keys');
-        $token->validate('abc');
+        $token->validate(OidcJwtHelper::defaultProviderSettings());
     }
 
     public function test_error_thrown_if_token_signature_not_validated_from_non_matching_key()
@@ -83,7 +83,7 @@ class OidcIdTokenTest extends TestCase
         ]);
         $this->expectException(OidcInvalidTokenException::class);
         $this->expectExceptionMessage('Token signature could not be validated using the provided keys');
-        $token->validate('abc');
+        $token->validate(OidcJwtHelper::defaultProviderSettings());
     }
 
     public function test_error_thrown_if_invalid_key_provided()
@@ -91,15 +91,34 @@ class OidcIdTokenTest extends TestCase
         $token = new OidcIdToken(OidcJwtHelper::idToken(), OidcJwtHelper::defaultIssuer(), ['url://example.com']);
         $this->expectException(OidcInvalidTokenException::class);
         $this->expectExceptionMessage('Unexpected type of key value provided');
-        $token->validate('abc');
+        $token->validate(OidcJwtHelper::defaultProviderSettings());
     }
 
-    public function test_error_thrown_if_token_algorithm_is_not_rs256()
+    public function test_error_thrown_if_token_algorithm_is_not_supported()
     {
-        $token = new OidcIdToken(OidcJwtHelper::idToken([], ['alg' => 'HS256']), OidcJwtHelper::defaultIssuer(), []);
+        $token = new OidcIdToken(OidcJwtHelper::idToken([], ['alg' => 'ES256']), OidcJwtHelper::defaultIssuer(), []);
         $this->expectException(OidcInvalidTokenException::class);
-        $this->expectExceptionMessage('Only RS256 signature validation is supported. Token reports using HS256');
+        $this->expectExceptionMessage('Only HS256, RS256 signatures validation are supported. Token reports using ES256');
-        $token->validate('abc');
+        $token->validate(OidcJwtHelper::defaultProviderSettings());
+    }
+
+    public function test_hs256_token_passes_validation_with_correct_secret()
+    {
+        $secret = OidcJwtHelper::defaultSecret();
+        $token = new OidcIdToken(OidcJwtHelper::hs256IdToken($secret), OidcJwtHelper::defaultIssuer(), []);
+        $settings = OidcJwtHelper::defaultProviderSettings(['clientSecret' => $secret]);
+
+        $this->assertTrue($token->validate($settings));
+    }
+
+    public function test_hs256_token_fails_validation_with_wrong_secret()
+    {
+        $token = new OidcIdToken(OidcJwtHelper::hs256IdToken('correct-secret'), OidcJwtHelper::defaultIssuer(), []);
+        $settings = OidcJwtHelper::defaultProviderSettings(['clientSecret' => 'wrong-secret']);
+
+        $this->expectException(OidcInvalidTokenException::class);
+        $this->expectExceptionMessage('Token signature could not be validated using the provided secret');
+        $token->validate($settings);
     }
 
     public function test_token_claim_error_cases()
@@ -141,7 +160,7 @@ class OidcIdTokenTest extends TestCase
             $err = null;
 
             try {
-                $token->validate('xxyyzz.aaa.bbccdd.123');
+                $token->validate(OidcJwtHelper::defaultProviderSettings());
             } catch (\Exception $exception) {
                 $err = $exception;
             }
@@ -160,7 +179,7 @@ class OidcIdTokenTest extends TestCase
             $testFilePath,
         ]);
 
-        $this->assertTrue($token->validate('xxyyzz.aaa.bbccdd.123'));
+        $this->assertTrue($token->validate(OidcJwtHelper::defaultProviderSettings()));
         unlink($testFilePath);
     }
 }

tests/test-data/fonts/attribution.txt

@@ -1,2 +0,0 @@
-Font files by Roger White, in public domain.
-https://web.archive.org/web/20110609213636/http://www.rogersfonts.org.uk/