From 9321d08bf9443d6014a6cf80c207e6df35bc789b Mon Sep 17 00:00:00 2001 From: Ostap Brehin Date: Mon, 7 Sep 2026 00:29:55 +0100 Subject: [PATCH 1/2] feat(http)!: regenerate the session identifier on authentication --- .../AuthenticatorInitializer.php | 4 +- .../Authentication/SessionAuthenticator.php | 14 ++-- .../auth/tests/SessionAuthenticatorTest.php | 84 +++++++++++++++++-- .../Resolvers/CookieSessionIdResolver.php | 44 ++++++---- .../Resolvers/HeaderSessionIdResolver.php | 5 ++ packages/http/src/Session/Session.php | 15 ++++ .../http/src/Session/SessionIdResolver.php | 10 +++ .../http/src/Session/SessionRegenerator.php | 36 ++++++++ .../Http/SessionRegeneratorTest.php | 76 +++++++++++++++++ 9 files changed, 258 insertions(+), 30 deletions(-) create mode 100644 packages/http/src/Session/SessionRegenerator.php create mode 100644 tests/Integration/Http/SessionRegeneratorTest.php diff --git a/packages/auth/src/Authentication/AuthenticatorInitializer.php b/packages/auth/src/Authentication/AuthenticatorInitializer.php index e87555096b..92a86d549c 100644 --- a/packages/auth/src/Authentication/AuthenticatorInitializer.php +++ b/packages/auth/src/Authentication/AuthenticatorInitializer.php @@ -8,7 +8,7 @@ use Tempest\Container\Initializer; use Tempest\Container\Singleton; use Tempest\Http\Session\Session; -use Tempest\Http\Session\SessionManager; +use Tempest\Http\Session\SessionRegenerator; final readonly class AuthenticatorInitializer implements Initializer { @@ -16,9 +16,9 @@ public function initialize(Container $container): Authenticator { return new SessionAuthenticator( - sessionManager: $container->get(SessionManager::class), session: $container->get(Session::class), authenticatableResolver: $container->get(AuthenticatableResolver::class), + sessionRegenerator: $container->get(SessionRegenerator::class), ); } } diff --git a/packages/auth/src/Authentication/SessionAuthenticator.php b/packages/auth/src/Authentication/SessionAuthenticator.php index 572b4fcfef..ddf52330ea 100644 --- a/packages/auth/src/Authentication/SessionAuthenticator.php +++ b/packages/auth/src/Authentication/SessionAuthenticator.php @@ -5,7 +5,7 @@ namespace Tempest\Auth\Authentication; use Tempest\Http\Session\Session; -use Tempest\Http\Session\SessionManager; +use Tempest\Http\Session\SessionRegenerator; final class SessionAuthenticator implements Authenticator { @@ -20,9 +20,9 @@ final class SessionAuthenticator implements Authenticator private ?Authenticatable $current = null; public function __construct( - private readonly SessionManager $sessionManager, private readonly Session $session, private readonly AuthenticatableResolver $authenticatableResolver, + private readonly SessionRegenerator $sessionRegenerator, ) {} public function authenticate(Authenticatable $authenticatable): void @@ -43,15 +43,19 @@ public function authenticate(Authenticatable $authenticatable): void $this->currentId = $id; $this->currentClass = $class; $this->current = $authenticatable; + + // The session identifier must not survive a change in privilege level, or one + // known to an attacker before authentication stays valid afterwards. + $this->sessionRegenerator->regenerate(); } public function deauthenticate(): void { - $this->session->remove(self::AUTHENTICATABLE_KEY); - $this->session->remove(self::AUTHENTICATABLE_CLASS); $this->clearCurrent(); - $this->sessionManager->save($this->session); + // Regenerate session without preserving data to prevent session fixation + // and purge all authenticated user data. + $this->sessionRegenerator->regenerate(preserveData: false); } public function current(): ?Authenticatable diff --git a/packages/auth/tests/SessionAuthenticatorTest.php b/packages/auth/tests/SessionAuthenticatorTest.php index 6e791e796d..88b4f856ca 100644 --- a/packages/auth/tests/SessionAuthenticatorTest.php +++ b/packages/auth/tests/SessionAuthenticatorTest.php @@ -13,7 +13,9 @@ use Tempest\DateTime\DateTime; use Tempest\Http\Session\Session; use Tempest\Http\Session\SessionId; +use Tempest\Http\Session\SessionIdResolver; use Tempest\Http\Session\SessionManager; +use Tempest\Http\Session\SessionRegenerator; final class SessionAuthenticatorTest extends TestCase { @@ -27,9 +29,9 @@ public function current_memoizes_the_resolved_authenticatable_for_the_current_se $session->set(SessionAuthenticator::AUTHENTICATABLE_CLASS, MemoizedAuthenticatable::class); $authenticator = new SessionAuthenticator( - sessionManager: new TestingSessionManager(), session: $session, authenticatableResolver: $resolver, + sessionRegenerator: $this->createRegenerator($session), ); $this->assertSame($authenticatable, $authenticator->current()); @@ -46,9 +48,9 @@ public function current_memoizes_a_missing_authenticatable_for_the_current_sessi $session->set(SessionAuthenticator::AUTHENTICATABLE_CLASS, MemoizedAuthenticatable::class); $authenticator = new SessionAuthenticator( - sessionManager: new TestingSessionManager(), session: $session, authenticatableResolver: $resolver, + sessionRegenerator: $this->createRegenerator($session), ); $this->assertNull($authenticator->current()); @@ -68,9 +70,9 @@ public function current_re_resolves_when_the_session_identity_changes(): void $session->set(SessionAuthenticator::AUTHENTICATABLE_CLASS, MemoizedAuthenticatable::class); $authenticator = new SessionAuthenticator( - sessionManager: new TestingSessionManager(), session: $session, authenticatableResolver: $resolver, + sessionRegenerator: $this->createRegenerator($session), ); $current = $authenticator->current(); @@ -95,9 +97,9 @@ public function reset_clears_the_cached_current_authenticatable(): void $session->set(SessionAuthenticator::AUTHENTICATABLE_CLASS, MemoizedAuthenticatable::class); $authenticator = new SessionAuthenticator( - sessionManager: new TestingSessionManager(), session: $session, authenticatableResolver: $resolver, + sessionRegenerator: $this->createRegenerator($session), ); $this->assertSame($authenticatable, $authenticator->current()); @@ -120,9 +122,9 @@ public function authenticate_replaces_a_cached_current_authenticatable(): void $session->set(SessionAuthenticator::AUTHENTICATABLE_CLASS, MemoizedAuthenticatable::class); $authenticator = new SessionAuthenticator( - sessionManager: new TestingSessionManager(), session: $session, authenticatableResolver: $resolver, + sessionRegenerator: $this->createRegenerator($session), ); $current = $authenticator->current(); @@ -136,6 +138,58 @@ public function authenticate_replaces_a_cached_current_authenticatable(): void $this->assertSame(2, $current->id); } + #[Test] + public function authenticate_regenerates_the_session_identifier(): void + { + $session = $this->createSession(); + $sessionManager = new TestingSessionManager(); + + $authenticator = new SessionAuthenticator( + session: $session, + authenticatableResolver: new CountingAuthenticatableResolver(), + sessionRegenerator: $this->createRegenerator($session, $sessionManager), + ); + + $authenticator->authenticate(new MemoizedAuthenticatable(id: 1)); + + $this->assertNotSame('test-session', (string) $session->id); + $this->assertSame(1, $sessionManager->deletedSessions); + $this->assertSame(1, $session->get(SessionAuthenticator::AUTHENTICATABLE_KEY)); + } + + #[Test] + public function deauthenticate_regenerates_the_session_identifier_and_discards_the_data(): void + { + $session = $this->createSession(); + $session->set(SessionAuthenticator::AUTHENTICATABLE_KEY, 1); + $session->set(SessionAuthenticator::AUTHENTICATABLE_CLASS, MemoizedAuthenticatable::class); + $session->set('key', 'value'); + $sessionManager = new TestingSessionManager(); + + $authenticator = new SessionAuthenticator( + session: $session, + authenticatableResolver: new CountingAuthenticatableResolver(), + sessionRegenerator: $this->createRegenerator($session, $sessionManager), + ); + + $authenticator->deauthenticate(); + + $this->assertNotSame('test-session', (string) $session->id); + $this->assertSame(1, $sessionManager->deletedSessions); + $this->assertNull($session->get(SessionAuthenticator::AUTHENTICATABLE_KEY)); + $this->assertNull($session->get(SessionAuthenticator::AUTHENTICATABLE_CLASS)); + $this->assertNull($session->get('key')); + } + + private function createRegenerator(Session $session, ?SessionManager $sessionManager = null): SessionRegenerator + { + return new SessionRegenerator( + sessionManager: $sessionManager ?? new TestingSessionManager(), + session: $session, + sessionIdResolver: new TestingSessionIdResolver(), + ); + } + private function createSession(): Session { $now = DateTime::now(); @@ -186,10 +240,25 @@ public function resolveId(Authenticatable $authenticatable): int } } +final class TestingSessionIdResolver implements SessionIdResolver +{ + public function resolve(): SessionId + { + return new SessionId('test-session'); + } + + public function regenerate(): SessionId + { + return new SessionId('regenerated-session-' . uniqid()); + } +} + final class TestingSessionManager implements SessionManager { public int $savedSessions = 0; + public int $deletedSessions = 0; + public function getOrCreate(SessionId $id): Session { $now = DateTime::now(); @@ -202,7 +271,10 @@ public function save(Session $session): void $this->savedSessions++; } - public function delete(Session $session): void {} + public function delete(Session $session): void + { + $this->deletedSessions++; + } public function isValid(Session $session): bool { diff --git a/packages/http/src/Session/Resolvers/CookieSessionIdResolver.php b/packages/http/src/Session/Resolvers/CookieSessionIdResolver.php index 4c5077ac76..d92307fb6e 100644 --- a/packages/http/src/Session/Resolvers/CookieSessionIdResolver.php +++ b/packages/http/src/Session/Resolvers/CookieSessionIdResolver.php @@ -30,27 +30,37 @@ public function __construct( public function resolve(): SessionId { - $sessionKey = str($this->appConfig->name ?? 'tempest') - ->snake() - ->append('_session_id') - ->toString(); - - $id = $this->request->getCookie($sessionKey)?->value; + $id = $this->request->getCookie($this->getSessionKey())?->value; if (! $id) { - $id = (string) Uuid::v4(); - - $this->cookies->add(new Cookie( - key: $sessionKey, - value: $id, - expiresAt: $this->clock->now()->plus($this->sessionConfig->expiration), - path: '/', - secure: Str\starts_with($this->appConfig->baseUri, needles: 'https'), - httpOnly: true, - sameSite: SameSite::LAX, - )); + return $this->regenerate(); } return new SessionId($id); } + + public function regenerate(): SessionId + { + $id = (string) Uuid::v4(); + + $this->cookies->add(new Cookie( + key: $this->getSessionKey(), + value: $id, + expiresAt: $this->clock->now()->plus($this->sessionConfig->expiration), + path: '/', + secure: Str\starts_with($this->appConfig->baseUri, needles: 'https'), + httpOnly: true, + sameSite: SameSite::LAX, + )); + + return new SessionId($id); + } + + private function getSessionKey(): string + { + return str($this->appConfig->name ?? 'tempest') + ->snake() + ->append('_session_id') + ->toString(); + } } diff --git a/packages/http/src/Session/Resolvers/HeaderSessionIdResolver.php b/packages/http/src/Session/Resolvers/HeaderSessionIdResolver.php index ecc26b7245..81163fe5f0 100644 --- a/packages/http/src/Session/Resolvers/HeaderSessionIdResolver.php +++ b/packages/http/src/Session/Resolvers/HeaderSessionIdResolver.php @@ -30,4 +30,9 @@ public function resolve(): SessionId id: $this->request->headers[$sessionKey] ?? Uuid::v4()->toString(), ); } + + public function regenerate(): SessionId + { + return new SessionId(id: Uuid::v4()->toString()); + } } diff --git a/packages/http/src/Session/Session.php b/packages/http/src/Session/Session.php index 78846c4a92..da1ad34cbb 100644 --- a/packages/http/src/Session/Session.php +++ b/packages/http/src/Session/Session.php @@ -132,6 +132,21 @@ public function cleanup(): void } } + /** + * Assigns a new identifier to the session, optionally discarding all its data. + * + * Prefer {@see SessionRegenerator}, which also destroys the session that is being + * replaced and sends the new identifier to the client. + */ + public function replaceId(SessionId $id, bool $preserveData = true): void + { + $this->id = $id; + + if (! $preserveData) { + $this->clear(); + } + } + /** * Clears all values from the session. */ diff --git a/packages/http/src/Session/SessionIdResolver.php b/packages/http/src/Session/SessionIdResolver.php index 6b49d69ddb..58114d6726 100644 --- a/packages/http/src/Session/SessionIdResolver.php +++ b/packages/http/src/Session/SessionIdResolver.php @@ -6,5 +6,15 @@ interface SessionIdResolver { + /** + * Resolves the identifier sent by the client, creating a new one if there is none. + */ public function resolve(): SessionId; + + /** + * Creates a new identifier and sends it to the client, replacing the one it was using. + * + * @see SessionRegenerator + */ + public function regenerate(): SessionId; } diff --git a/packages/http/src/Session/SessionRegenerator.php b/packages/http/src/Session/SessionRegenerator.php new file mode 100644 index 0000000000..6453c13d48 --- /dev/null +++ b/packages/http/src/Session/SessionRegenerator.php @@ -0,0 +1,36 @@ +sessionManager->delete($this->session); + + $this->session->replaceId( + id: $this->sessionIdResolver->regenerate(), + preserveData: $preserveData, + ); + + $this->sessionManager->save($this->session); + } +} diff --git a/tests/Integration/Http/SessionRegeneratorTest.php b/tests/Integration/Http/SessionRegeneratorTest.php new file mode 100644 index 0000000000..07498dd1bd --- /dev/null +++ b/tests/Integration/Http/SessionRegeneratorTest.php @@ -0,0 +1,76 @@ + $this->container->get(Session::class); + } + + private SessionRegenerator $regenerator { + get => $this->container->get(SessionRegenerator::class); + } + + #[Test] + public function assigns_a_new_identifier_and_keeps_data(): void + { + $this->session->set('key', 'value'); + $previousId = (string) $this->session->id; + + $this->regenerator->regenerate(); + + $this->assertNotSame($previousId, (string) $this->session->id); + $this->assertSame('value', $this->session->get('key')); + } + + #[Test] + public function discards_data_when_it_is_not_preserved(): void + { + $this->session->set('key', 'value'); + + $this->regenerator->regenerate(preserveData: false); + + $this->assertNull($this->session->get('key')); + } + + #[Test] + public function destroys_the_session_it_replaces(): void + { + $sessionManager = $this->container->get(SessionManager::class); + + $this->session->set('key', 'value'); + $previousId = $this->session->id; + + $this->regenerator->regenerate(); + + $previousSession = $sessionManager->getOrCreate($previousId); + + $this->assertNull($previousSession->get('key')); + } + + #[Test] + public function sends_the_new_identifier_to_the_client(): void + { + $cookies = $this->container->get(CookieManager::class); + + $this->regenerator->regenerate(); + + $this->assertSame( + (string) $this->session->id, + $cookies->get('tempest_session_id')?->value, + ); + } +} From 364650cf5c40c7dec5eaf4bfc4b6c2add6da3a3e Mon Sep 17 00:00:00 2001 From: Enzo Innocenzi Date: Mon, 7 Sep 2026 18:51:02 +0200 Subject: [PATCH 2/2] refactor: clean up public api --- .../Authentication/SessionAuthenticator.php | 2 +- .../auth/tests/SessionAuthenticatorTest.php | 2 +- .../Resolvers/CookieSessionIdResolver.php | 4 ++-- .../Resolvers/HeaderSessionIdResolver.php | 2 +- packages/http/src/Session/Session.php | 10 ++------ .../http/src/Session/SessionIdResolver.php | 2 +- .../http/src/Session/SessionRegenerator.php | 23 +++++++++++++------ .../Http/SessionRegeneratorTest.php | 4 ++-- tests/Integration/Http/SessionTest.php | 12 ++++++++++ 9 files changed, 38 insertions(+), 23 deletions(-) diff --git a/packages/auth/src/Authentication/SessionAuthenticator.php b/packages/auth/src/Authentication/SessionAuthenticator.php index ddf52330ea..cc09a6cc39 100644 --- a/packages/auth/src/Authentication/SessionAuthenticator.php +++ b/packages/auth/src/Authentication/SessionAuthenticator.php @@ -55,7 +55,7 @@ public function deauthenticate(): void // Regenerate session without preserving data to prevent session fixation // and purge all authenticated user data. - $this->sessionRegenerator->regenerate(preserveData: false); + $this->sessionRegenerator->invalidate(); } public function current(): ?Authenticatable diff --git a/packages/auth/tests/SessionAuthenticatorTest.php b/packages/auth/tests/SessionAuthenticatorTest.php index 88b4f856ca..45dd4e459a 100644 --- a/packages/auth/tests/SessionAuthenticatorTest.php +++ b/packages/auth/tests/SessionAuthenticatorTest.php @@ -247,7 +247,7 @@ public function resolve(): SessionId return new SessionId('test-session'); } - public function regenerate(): SessionId + public function issueNewId(): SessionId { return new SessionId('regenerated-session-' . uniqid()); } diff --git a/packages/http/src/Session/Resolvers/CookieSessionIdResolver.php b/packages/http/src/Session/Resolvers/CookieSessionIdResolver.php index d92307fb6e..25b8831ff8 100644 --- a/packages/http/src/Session/Resolvers/CookieSessionIdResolver.php +++ b/packages/http/src/Session/Resolvers/CookieSessionIdResolver.php @@ -33,13 +33,13 @@ public function resolve(): SessionId $id = $this->request->getCookie($this->getSessionKey())?->value; if (! $id) { - return $this->regenerate(); + return $this->issueNewId(); } return new SessionId($id); } - public function regenerate(): SessionId + public function issueNewId(): SessionId { $id = (string) Uuid::v4(); diff --git a/packages/http/src/Session/Resolvers/HeaderSessionIdResolver.php b/packages/http/src/Session/Resolvers/HeaderSessionIdResolver.php index 81163fe5f0..765c3513c9 100644 --- a/packages/http/src/Session/Resolvers/HeaderSessionIdResolver.php +++ b/packages/http/src/Session/Resolvers/HeaderSessionIdResolver.php @@ -31,7 +31,7 @@ public function resolve(): SessionId ); } - public function regenerate(): SessionId + public function issueNewId(): SessionId { return new SessionId(id: Uuid::v4()->toString()); } diff --git a/packages/http/src/Session/Session.php b/packages/http/src/Session/Session.php index da1ad34cbb..677485a66f 100644 --- a/packages/http/src/Session/Session.php +++ b/packages/http/src/Session/Session.php @@ -133,18 +133,12 @@ public function cleanup(): void } /** - * Assigns a new identifier to the session, optionally discarding all its data. - * - * Prefer {@see SessionRegenerator}, which also destroys the session that is being + * @internal Prefer {@see SessionRegenerator}, which also destroys the session that is being * replaced and sends the new identifier to the client. */ - public function replaceId(SessionId $id, bool $preserveData = true): void + public function replaceId(SessionId $id): void { $this->id = $id; - - if (! $preserveData) { - $this->clear(); - } } /** diff --git a/packages/http/src/Session/SessionIdResolver.php b/packages/http/src/Session/SessionIdResolver.php index 58114d6726..aa52293593 100644 --- a/packages/http/src/Session/SessionIdResolver.php +++ b/packages/http/src/Session/SessionIdResolver.php @@ -16,5 +16,5 @@ public function resolve(): SessionId; * * @see SessionRegenerator */ - public function regenerate(): SessionId; + public function issueNewId(): SessionId; } diff --git a/packages/http/src/Session/SessionRegenerator.php b/packages/http/src/Session/SessionRegenerator.php index 6453c13d48..9ee2afa123 100644 --- a/packages/http/src/Session/SessionRegenerator.php +++ b/packages/http/src/Session/SessionRegenerator.php @@ -19,17 +19,26 @@ public function __construct( ) {} /** - * Assigns a new ID to the current session, carrying over data by default. + * Assigns a new ID to the current session, carrying over data. */ - public function regenerate(bool $preserveData = true): void + public function regenerate(): void { - // Destroy the old session to prevent parallel active sessions. $this->sessionManager->delete($this->session); - $this->session->replaceId( - id: $this->sessionIdResolver->regenerate(), - preserveData: $preserveData, - ); + $this->session->replaceId($this->sessionIdResolver->issueNewId()); + + $this->sessionManager->save($this->session); + } + + /** + * Assigns a new ID to the current session, discarding all data. + */ + public function invalidate(): void + { + $this->sessionManager->delete($this->session); + + $this->session->replaceId($this->sessionIdResolver->issueNewId()); + $this->session->clear(); $this->sessionManager->save($this->session); } diff --git a/tests/Integration/Http/SessionRegeneratorTest.php b/tests/Integration/Http/SessionRegeneratorTest.php index 07498dd1bd..9518b6e099 100644 --- a/tests/Integration/Http/SessionRegeneratorTest.php +++ b/tests/Integration/Http/SessionRegeneratorTest.php @@ -37,11 +37,11 @@ public function assigns_a_new_identifier_and_keeps_data(): void } #[Test] - public function discards_data_when_it_is_not_preserved(): void + public function invalidate_discards_data(): void { $this->session->set('key', 'value'); - $this->regenerator->regenerate(preserveData: false); + $this->regenerator->invalidate(); $this->assertNull($this->session->get('key')); } diff --git a/tests/Integration/Http/SessionTest.php b/tests/Integration/Http/SessionTest.php index e6d22a9107..7642f2a783 100644 --- a/tests/Integration/Http/SessionTest.php +++ b/tests/Integration/Http/SessionTest.php @@ -6,6 +6,7 @@ use PHPUnit\Framework\Attributes\Test; use Tempest\Http\Session\Session; +use Tempest\Http\Session\SessionId; use Tests\Tempest\Integration\FrameworkIntegrationTestCase; /** @@ -139,6 +140,17 @@ public function clear(): void $this->assertEmpty($this->session->all()); } + #[Test] + public function replace_id_preserves_data(): void + { + $this->session->set('key', 'value'); + + $this->session->replaceId(new SessionId('new_session')); + + $this->assertSame('new_session', (string) $this->session->id); + $this->assertSame('value', $this->session->get('key')); + } + #[Test] public function session_is_reset(): void {