Compare commits

...

5 Commits

Author SHA1 Message Date
Dan Brown
16110273ff Updated version and assets for release v25.11.5 2025-12-03 14:49:20 +00:00
Dan Brown
93bcbd168e Merge branch 'v25-11' into release 2025-12-03 14:32:56 +00:00
Dan Brown
65f7b61c1f Sessions: Ignored extra meta/dist content in history tracking
For #5925
Added tests to cover.
Extracted existing test to place with similiar sessions tests
2025-12-03 14:10:09 +00:00
Dan Brown
2fde803c76 Deps: Updated PHP package versions
Needed to update some tests due to charset casing change in Symfony 7.4
2025-12-03 13:55:00 +00:00
Dan Brown
adfac3e30e OIDC: Updated state handling to prevent loss from other requests
Which was occuring in chrome, where background requests to the PWA
manifest, or opensearch, endpoint caused OIDC to fail due to lost state
since it was only flashed to the session.
This persists it with a manual TTL.

Added tests to cover.
Manually tested against Azure.
For #5929
2025-12-03 13:34:00 +00:00
16 changed files with 398 additions and 319 deletions

View File

@@ -9,11 +9,9 @@ use Illuminate\Http\Request;
class OidcController extends Controller
{
protected OidcService $oidcService;
public function __construct(OidcService $oidcService)
{
$this->oidcService = $oidcService;
public function __construct(
protected 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()
{

View File

@@ -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',
];
/**

539
composer.lock generated

File diff suppressed because it is too large Load Diff

View File

@@ -1 +1 @@
ea202c9927a6b6b0bb9f30969642127b6fa14cf5ce70e56482b84ffd5bcf92bf
f685179ac59c80d2b635c1df33974830616d7e055fc390be1a17056443644e4a

2
public/dist/app.js vendored

File diff suppressed because one or more lines are too long

12
public/dist/code.js vendored

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

View File

@@ -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()

View File

@@ -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);

View File

@@ -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');
}

53
tests/SessionTest.php Normal file
View File

@@ -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());
}
}

View File

@@ -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');
});
}

View File

@@ -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();

View File

@@ -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');

View File

@@ -1 +1 @@
v25.11.4
v25.11.5