From 4a1820767a6c46e4028f87f390586278bf359a44 Mon Sep 17 00:00:00 2001 From: Michal Izewski Date: Tue, 7 May 2024 11:23:03 +0200 Subject: [PATCH 1/7] Add auto_regenerate option to CacheSessionPersistence Signed-off-by: Michal Izewski --- docs/book/v1/manual.md | 8 ++++++-- src/CacheSessionPersistence.php | 12 +++++++++--- src/CacheSessionPersistenceFactory.php | 4 +++- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/docs/book/v1/manual.md b/docs/book/v1/manual.md index 43bfdbb..7e1030a 100644 --- a/docs/book/v1/manual.md +++ b/docs/book/v1/manual.md @@ -36,7 +36,10 @@ The following details the constructor of the `Mezzio\Session\Cache\CacheSessionP * @param string $cookieSameSite The same-site rule to apply to the persisted * cookie. Options include "Lax", "Strict", and "None". * Available since 1.4.0 - * + * @param bool $autoRegenerate Whether or not the session ID should be + * regenerated on session data changes + * Available since 1.13.0 + * * @todo reorder these arguments so they make more sense and are in an * order of importance */ @@ -51,7 +54,8 @@ public function __construct( string $cookieDomain = null, bool $cookieSecure = false, bool $cookieHttpOnly = false, - string $cookieSameSite = 'Lax' + string $cookieSameSite = 'Lax', + bool $autoRegenerate = true, ) { ``` diff --git a/src/CacheSessionPersistence.php b/src/CacheSessionPersistence.php index 66f81ad..0183a48 100644 --- a/src/CacheSessionPersistence.php +++ b/src/CacheSessionPersistence.php @@ -34,6 +34,7 @@ class CacheSessionPersistence implements SessionPersistenceInterface private CacheItemPoolInterface $cache; private bool $persistent; + private bool $autoRegenerate; /** * Prepare session cache and default HTTP caching headers. @@ -67,6 +68,8 @@ class CacheSessionPersistence implements SessionPersistenceInterface * be accessed by client-side apis. * @param string $cookieSameSite The same-site rule to apply to the persisted * cookie. Options include "Lax", "Strict", and "None". + * @param bool $autoRegenerate Whether or not the session ID should be + * regenerated on session data changes * @todo reorder the constructor arguments */ public function __construct( @@ -80,7 +83,8 @@ public function __construct( ?string $cookieDomain = null, bool $cookieSecure = false, bool $cookieHttpOnly = false, - string $cookieSameSite = 'Lax' + string $cookieSameSite = 'Lax', + bool $autoRegenerate = true ) { $this->cache = $cache; @@ -112,6 +116,8 @@ public function __construct( : $this->getLastModified(); $this->persistent = $persistent; + + $this->autoRegenerate = $autoRegenerate; } public function initializeSessionFromRequest(ServerRequestInterface $request): SessionInterface @@ -139,8 +145,8 @@ public function persistSession(SessionInterface $session, ResponseInterface $res // Regenerate the session if: // - we have no session identifier // - the session is marked as regenerated - // - the session has changed (data is different) - if ('' === $id || $session->isRegenerated() || $session->hasChanged()) { + // - the session has changed (data is different) and autoRegenerate is turned off in the configuration + if ('' === $id || $session->isRegenerated() || ($this->autoRegenerate && $session->hasChanged())) { $id = $this->regenerateSession($id); } diff --git a/src/CacheSessionPersistenceFactory.php b/src/CacheSessionPersistenceFactory.php index 65a9e4c..800918f 100644 --- a/src/CacheSessionPersistenceFactory.php +++ b/src/CacheSessionPersistenceFactory.php @@ -34,6 +34,7 @@ public function __invoke(ContainerInterface $container) $cacheExpire = $config['cache_expire'] ?? 10800; $lastModified = $config['last_modified'] ?? null; $persistent = $config['persistent'] ?? false; + $autoRegenerate = $config['auto_regenerate'] ?? true; return new CacheSessionPersistence( $container->get($cacheService), @@ -46,7 +47,8 @@ public function __invoke(ContainerInterface $container) $cookieDomain, $cookieSecure, $cookieHttpOnly, - $cookieSameSite + $cookieSameSite, + $autoRegenerate ); } } From c98471649c03f938f15e75f5c7740872d4932055 Mon Sep 17 00:00:00 2001 From: Michal Izewski Date: Tue, 7 May 2024 13:30:11 +0200 Subject: [PATCH 2/7] Update Psalm baseline with auto_regenerate option Signed-off-by: Michal Izewski --- psalm-baseline.xml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 3c353c0..3478d1a 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -29,6 +29,7 @@ $cookieSecure $lastModified $persistent + $autoRegenerate @@ -43,6 +44,7 @@ + $cacheExpire @@ -58,6 +60,7 @@ $cookieSecure $lastModified $persistent + $autoRegenerate From b1466feb3000815bdb0e5b85c5b92db42ad7ea35 Mon Sep 17 00:00:00 2001 From: Michal Izewski Date: Tue, 7 May 2024 13:36:41 +0200 Subject: [PATCH 3/7] Update PHPUnit tests for auto_regenerate option Signed-off-by: Michal Izewski --- test/CacheSessionPersistenceFactoryTest.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/CacheSessionPersistenceFactoryTest.php b/test/CacheSessionPersistenceFactoryTest.php index b288b65..96c675c 100644 --- a/test/CacheSessionPersistenceFactoryTest.php +++ b/test/CacheSessionPersistenceFactoryTest.php @@ -74,6 +74,7 @@ public function testFactoryUsesSaneDefaultsForConstructorArguments(): void $this->assertAttributeSame(10800, 'cacheExpire', $persistence); $this->assertAttributeNotEmpty('lastModified', $persistence); $this->assertAttributeSame(false, 'persistent', $persistence); + $this->assertAttributeSame(true, 'autoRegenerate', $persistence); } public function testFactoryAllowsConfiguringAllConstructorArguments(): void @@ -95,6 +96,7 @@ public function testFactoryAllowsConfiguringAllConstructorArguments(): void 'cache_expire' => 300, 'last_modified' => $lastModified, 'persistent' => true, + 'auto_regenerate' => false, ], ]); @@ -115,6 +117,7 @@ public function testFactoryAllowsConfiguringAllConstructorArguments(): void $persistence ); $this->assertAttributeSame(true, 'persistent', $persistence); + $this->assertAttributeSame(false, 'autoRegenerate', $persistence); } public function testFactoryAllowsConfiguringCacheAdapterServiceName(): void From 55400ebcf45bba25e30224b31e3753e563c8a3ec Mon Sep 17 00:00:00 2001 From: Michal Izewski Date: Tue, 7 May 2024 14:55:47 +0200 Subject: [PATCH 4/7] New test covering auto_regenerate option set to false Signed-off-by: Michal Izewski --- ...CacheSessionPersistenceIntegrationTest.php | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/test/CacheSessionPersistenceIntegrationTest.php b/test/CacheSessionPersistenceIntegrationTest.php index da85037..55b3101 100644 --- a/test/CacheSessionPersistenceIntegrationTest.php +++ b/test/CacheSessionPersistenceIntegrationTest.php @@ -111,4 +111,40 @@ public function testThatAChangedSessionWillCauseRegenerationAndASetCookieHeader( $value = $item->get(); self::assertNull($value, 'The previous session data should have been deleted'); } + + public function testThatAChangedSessionWillNotCauseRegenerationAndASetCookieHeader(): void + { + $this->storage = new CacheSessionPersistence( + $this->cache, + cookieName: 'Session', + autoRegenerate: false, + ); + + $item = $this->cache->getItem('foo'); + $item->set(['foo' => 'bar']); + $this->cache->save($item); + + $request = (new ServerRequest())->withHeader('Cookie', 'Session=foo;'); + $middleware = new SessionMiddleware($this->storage); + $handler = new TestHandler(); + $handler->setSessionVariable('something', 'groovy'); + $response = $middleware->process($request, $handler); + + $setCookie = SetCookies::fromResponse($response); + $cookie = $setCookie->get('Session'); + self::assertNotNull($cookie); + + $id = $cookie->getValue(); + self::assertNotNull($id); + + self::assertSame('foo', $id); + self::assertNotSame($handler->response, $response); + + $item = $this->cache->getItem('foo'); + $value = $item->get(); + self::assertSame([ + 'foo' => 'bar', + 'something' => 'groovy' + ],$value, 'The session data should have been updated'); + } } From 9a76d8cb6d338da318d6b33d2db2736fbe6bd5e5 Mon Sep 17 00:00:00 2001 From: Michal Izewski Date: Tue, 7 May 2024 15:00:54 +0200 Subject: [PATCH 5/7] Fix phpcs errors Signed-off-by: Michal Izewski --- test/CacheSessionPersistenceIntegrationTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/CacheSessionPersistenceIntegrationTest.php b/test/CacheSessionPersistenceIntegrationTest.php index 55b3101..388b5ea 100644 --- a/test/CacheSessionPersistenceIntegrationTest.php +++ b/test/CacheSessionPersistenceIntegrationTest.php @@ -143,8 +143,8 @@ public function testThatAChangedSessionWillNotCauseRegenerationAndASetCookieHead $item = $this->cache->getItem('foo'); $value = $item->get(); self::assertSame([ - 'foo' => 'bar', - 'something' => 'groovy' - ],$value, 'The session data should have been updated'); + 'foo' => 'bar', + 'something' => 'groovy', + ], $value, 'The session data should have been updated'); } } From e571fe6e584b9f8f82d4c54dbed75b28d2381004 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20I=C5=BCewski?= <50335909+michal-izewski@users.noreply.github.com> Date: Tue, 7 May 2024 16:54:11 +0200 Subject: [PATCH 6/7] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: George Steel Signed-off-by: Michał Iżewski <50335909+michal-izewski@users.noreply.github.com> --- src/CacheSessionPersistence.php | 2 +- test/CacheSessionPersistenceIntegrationTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/CacheSessionPersistence.php b/src/CacheSessionPersistence.php index 0183a48..13f3c27 100644 --- a/src/CacheSessionPersistence.php +++ b/src/CacheSessionPersistence.php @@ -145,7 +145,7 @@ public function persistSession(SessionInterface $session, ResponseInterface $res // Regenerate the session if: // - we have no session identifier // - the session is marked as regenerated - // - the session has changed (data is different) and autoRegenerate is turned off in the configuration + // - the session has changed (data is different) and autoRegenerate is turned on (default) in the configuration if ('' === $id || $session->isRegenerated() || ($this->autoRegenerate && $session->hasChanged())) { $id = $this->regenerateSession($id); } diff --git a/test/CacheSessionPersistenceIntegrationTest.php b/test/CacheSessionPersistenceIntegrationTest.php index 388b5ea..e56b491 100644 --- a/test/CacheSessionPersistenceIntegrationTest.php +++ b/test/CacheSessionPersistenceIntegrationTest.php @@ -112,7 +112,7 @@ public function testThatAChangedSessionWillCauseRegenerationAndASetCookieHeader( self::assertNull($value, 'The previous session data should have been deleted'); } - public function testThatAChangedSessionWillNotCauseRegenerationAndASetCookieHeader(): void + public function testThatAChangedSessionWillNotCauseRegenerationWhenAutoRegenerateIsFalse(): void { $this->storage = new CacheSessionPersistence( $this->cache, From c78750db72d6fac42b8c98e34a38f4ad98f4bf46 Mon Sep 17 00:00:00 2001 From: Michal Izewski Date: Tue, 7 May 2024 19:50:27 +0200 Subject: [PATCH 7/7] Type casting for boolean arguments Signed-off-by: Michal Izewski --- psalm-baseline.xml | 4 ---- src/CacheSessionPersistenceFactory.php | 8 ++++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 3478d1a..4d68d07 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -22,14 +22,10 @@ $cacheService get($cacheService)]]> $cookieDomain - $cookieHttpOnly $cookieName $cookiePath $cookieSameSite - $cookieSecure $lastModified - $persistent - $autoRegenerate diff --git a/src/CacheSessionPersistenceFactory.php b/src/CacheSessionPersistenceFactory.php index 800918f..b2e4654 100644 --- a/src/CacheSessionPersistenceFactory.php +++ b/src/CacheSessionPersistenceFactory.php @@ -43,12 +43,12 @@ public function __invoke(ContainerInterface $container) $cacheLimiter, $cacheExpire, $lastModified, - $persistent, + (bool) $persistent, $cookieDomain, - $cookieSecure, - $cookieHttpOnly, + (bool) $cookieSecure, + (bool) $cookieHttpOnly, $cookieSameSite, - $autoRegenerate + (bool) $autoRegenerate ); } }