Merge branch 'v25-11' into release

app/Access/Controllers/OidcController.php

@@ -9,11 +9,9 @@ use Illuminate\Http\Request;
 
 class OidcController extends Controller
 {
-    protected OidcService $oidcService;
+    public function __construct(
-
+        protected OidcService $oidcService
-    public function __construct(OidcService $oidcService)
+    ) {
-    {
-        $this->oidcService = $oidcService;
         $this->middleware('guard:oidc');
     }
 
@@ -30,7 +28,7 @@ class OidcController extends Controller
             return redirect('/login');
         }
 
-        session()->flash('oidc_state', $loginDetails['state']);
+        session()->put('oidc_state', time() . ':' . $loginDetails['state']);
 
         return redirect($loginDetails['url']);
     }
@@ -41,10 +39,16 @@ 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];
+        }
 
-        if ($storedState !== $responseState) {
+        [$storedStateTime, $storedState] = $splitState;
+        $threeMinutesAgo = time() - 3 * 60;
+
+        if (!$storedState || $storedState !== $responseState || intval($storedStateTime) < $threeMinutesAgo) {
             $this->showErrorNotification(trans('errors.oidc_fail_authed', ['system' => config('oidc.name')]));
 
             return redirect('/login');
@@ -62,7 +66,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/Http/Middleware/StartSessionExtended.php

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

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 = session()->get('oidc_state');
+        $state = explode(':', session()->get('oidc_state'), 2)[1];
 
         $transactions = $this->mockHttpClient([$this->getMockAuthorizationResponse([
             'email' => 'benny@example.com',
@@ -190,6 +190,35 @@ 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);
@@ -797,7 +826,7 @@ class OidcTest extends TestCase
     {
         // Start auth
         $resp = $this->post('/oidc/login');
-        $state = session()->get('oidc_state');
+        $state = explode(':', session()->get('oidc_state'), 2)[1];
 
         $pkceCode = session()->get('oidc_pkce_code');
         $this->assertGreaterThan(30, strlen($pkceCode));
@@ -825,7 +854,7 @@ class OidcTest extends TestCase
     {
         config()->set('oidc.display_name_claims', 'first_name|last_name');
         $this->post('/oidc/login');
-        $state = session()->get('oidc_state');
+        $state = explode(':', session()->get('oidc_state'), 2)[1];
 
         $client = $this->mockHttpClient([
             $this->getMockAuthorizationResponse(['name' => null]),
@@ -973,7 +1002,7 @@ class OidcTest extends TestCase
         ]);
 
         $this->post('/oidc/login');
-        $state = session()->get('oidc_state');
+        $state = explode(':', session()->get('oidc_state'), 2)[1];
         $client = $this->mockHttpClient([$this->getMockAuthorizationResponse([
             'groups' => [],
         ])]);
@@ -999,7 +1028,7 @@ class OidcTest extends TestCase
     protected function runLogin($claimOverrides = [], $additionalHttpResponses = []): TestResponse
     {
         $this->post('/oidc/login');
-        $state = session()->get('oidc_state');
+        $state = explode(':', session()->get('oidc_state'), 2)[1] ?? '';
         $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/SessionTest.php

@@ -0,0 +1,53 @@
+<?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

@@ -429,29 +429,6 @@ 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');