From d7c3784089284fc7183569f3bfc40ce3b950c198 Mon Sep 17 00:00:00 2001 From: Ralf Lang Date: Thu, 11 Jun 2026 23:10:31 +0200 Subject: [PATCH 1/5] fix(jwt): wire JwtService autowiring for modern routes Routes that bypass HordeCore middleware (JwtSessionLoader, HordeSessionMiddleware standalone) cannot resolve Horde\Core\Auth\Jwt\JwtService through DI today. The class has no factory attribute. The injector falls back to autowiring and fails on the constructor's string $secret parameter. Add #[Factory(JwtServiceFactory::create)] to JwtService. The factory already exists and reads conf, secret_file, issuer, ttls. It returns null when JWT is not enabled, which is correct for installs that have no JWT configuration. JwtSessionLoader's constructor required non-nullable JwtService. Made it ?JwtService and short-circuit resolveSession when null. A no-JWT install now passes the middleware through cleanly. The HordeSessionMiddleware downstream still mints a fresh session via the cookie path. --- src/Auth/Jwt/JwtService.php | 2 ++ src/Middleware/JwtSessionLoader.php | 9 ++++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/Auth/Jwt/JwtService.php b/src/Auth/Jwt/JwtService.php index 4f4b2c64..f1240af7 100644 --- a/src/Auth/Jwt/JwtService.php +++ b/src/Auth/Jwt/JwtService.php @@ -20,8 +20,10 @@ namespace Horde\Core\Auth\Jwt; +use Horde\Injector\Attribute\Factory; use InvalidArgumentException; +#[Factory(factory: JwtServiceFactory::class, method: 'create')] class JwtService { private Hs256Generator $generator; diff --git a/src/Middleware/JwtSessionLoader.php b/src/Middleware/JwtSessionLoader.php index a15a03a1..444ca429 100644 --- a/src/Middleware/JwtSessionLoader.php +++ b/src/Middleware/JwtSessionLoader.php @@ -64,7 +64,7 @@ class JwtSessionLoader implements MiddlewareInterface public const COOKIE_NAME = 'horde_jwt_refresh'; public function __construct( - private readonly JwtService $jwtService, + private readonly ?JwtService $jwtService, private readonly SessionHandler $sessionHandler, private readonly LoggerInterface $logger, ) {} @@ -90,6 +90,13 @@ public function process( */ private function resolveSession(ServerRequestInterface $request): ?HordeSession { + if ($this->jwtService === null) { + // JWT is not configured for this install. Nothing to verify. + // Downstream middleware (e.g. HordeSessionMiddleware) will + // mint a fresh session via the cookie path. + return null; + } + $cookies = $request->getCookieParams(); $jwt = $cookies[self::COOKIE_NAME] ?? null; if (!is_string($jwt) || $jwt === '') { From 84662ccfc14847c7b54886dd8ca23ef27ff363e0 Mon Sep 17 00:00:00 2001 From: Ralf Lang Date: Thu, 11 Jun 2026 23:10:41 +0200 Subject: [PATCH 2/5] fix(config): autowire State outside HordeCore bootstrap LoggerConfig and other DI consumers ask for Horde\Core\Config\State by autowiring. State's constructor accepts ?array $conf and falls back to $GLOBALS['conf']. That fallback only works after HordeCore middleware has called Horde_Registry::appInit which writes the global. Modern PSR-15 routes that skip HordeCore see an empty global and the autowire throws Config neither passed nor available from global. Add StateFactory that prefers the global if already populated and otherwise builds via ConfigLoader::load('horde'). ConfigLoader reads conf.php directly from disk and does not depend on the global. Bind State::class to StateFactory in DefaultInjectorBindings. Existing legacy callers that construct State directly continue to work; they go through the constructor not the injector. Modern routes can now resolve LoggerInterface and other indirect State consumers without first having HordeCore populate globals. --- src/Config/StateFactory.php | 50 +++++++++++++++++++++++++++++++++ src/DefaultInjectorBindings.php | 3 ++ 2 files changed, 53 insertions(+) create mode 100644 src/Config/StateFactory.php diff --git a/src/Config/StateFactory.php b/src/Config/StateFactory.php new file mode 100644 index 00000000..6960405b --- /dev/null +++ b/src/Config/StateFactory.php @@ -0,0 +1,50 @@ +getInstance(ConfigLoader::class)->load('horde'); + } +} diff --git a/src/DefaultInjectorBindings.php b/src/DefaultInjectorBindings.php index cd990a90..99645ac7 100644 --- a/src/DefaultInjectorBindings.php +++ b/src/DefaultInjectorBindings.php @@ -22,6 +22,8 @@ use Horde\Core\Config\ConfigMetadataProvider; use Horde\Core\Config\Driver\DriverRepository; use Horde\Core\Config\RegistryConfigLoader; +use Horde\Core\Config\State; +use Horde\Core\Config\StateFactory; use Horde\Core\Editor\TinymcePageBinder; use Horde\Core\Factory\ApiRegistryFactory; use Horde\Core\Factory\ApplicationServiceFactory; @@ -265,6 +267,7 @@ public function register(Injector $injector): void JwtService::class => JwtServiceFactory::class, AuthenticationService::class => AuthenticationServiceFactory::class, ConfigLoader::class => ConfigLoaderFactory::class, + State::class => StateFactory::class, DriverRepository::class => DriverRepositoryFactory::class, ConfigMetadataProvider::class => ConfigMetadataProviderFactory::class, HordeDbService::class => DbServiceFactory::class, From 919daee731cf15bf3a5d30fdf6a5d02f221ac8de Mon Sep 17 00:00:00 2001 From: Ralf Lang Date: Thu, 11 Jun 2026 23:10:52 +0200 Subject: [PATCH 3/5] fix(session): type SessionLifecycle secret against an interface SessionLifecycle's fourth constructor argument was typed ?Horde_Secret_Cbc. The legacy DI binding 'Horde_Secret_Cbc' resolves to Horde_Core_Secret_Cbc which extends Horde_Core_Secret extends Horde_Secret. It does NOT extend Horde_Secret_Cbc. PHP's runtime type check rejected the injector-supplied instance. Introduce Horde\Core\Secret\SessionSecret. The interface declares the two methods SessionLifecycle actually calls: setKey and clearKey. Untyped signatures mirror Horde_Secret so existing implementations satisfy the contract verbatim. Horde_Core_Secret_Cbc declares implements SessionSecret. No method body changes; its inherited setKey and clearKey already match. SessionLifecycle's constructor and PHPDoc retyped to ?SessionSecret. SessionLifecycleFactory still resolves the legacy 'Horde_Secret_Cbc' binding name and now checks instanceof SessionSecret. If the bound instance does not satisfy the contract (test fixtures, future overrides) the lifecycle is built without a secret and no-ops the rekey calls. --- lib/Horde/Core/Secret/Cbc.php | 3 +- src/Secret/SessionSecret.php | 63 +++++++++++++++++++++++++ src/Session/SessionLifecycle.php | 12 ++--- src/Session/SessionLifecycleFactory.php | 25 ++++++---- 4 files changed, 88 insertions(+), 15 deletions(-) create mode 100644 src/Secret/SessionSecret.php diff --git a/lib/Horde/Core/Secret/Cbc.php b/lib/Horde/Core/Secret/Cbc.php index 3fa06a88..1a58ac49 100644 --- a/lib/Horde/Core/Secret/Cbc.php +++ b/lib/Horde/Core/Secret/Cbc.php @@ -12,6 +12,7 @@ * @package Core */ +use Horde\Core\Secret\SessionSecret; use Horde\Crypt\Blowfish\Blowfish; /** @@ -32,7 +33,7 @@ * @package Core * @since 2.20.0 */ -class Horde_Core_Secret_Cbc extends Horde_Core_Secret +class Horde_Core_Secret_Cbc extends Horde_Core_Secret implements SessionSecret { /** * Key used for current cached cipher object. diff --git a/src/Secret/SessionSecret.php b/src/Secret/SessionSecret.php new file mode 100644 index 00000000..38d008b0 --- /dev/null +++ b/src/Secret/SessionSecret.php @@ -0,0 +1,63 @@ +getInstance('Horde_Secret_Cbc'); + $resolved = $injector->getInstance('Horde_Secret_Cbc'); + if ($resolved instanceof SessionSecret) { + $secret = $resolved; + } + // Resolved-but-not-SessionSecret: legacy fixture or test + // double. Treat as if no cipher were available; lifecycle + // no-ops the setKey/clearKey calls. } catch (\Throwable) { // No Horde_Secret_Cbc binding configured. Tests and // bootstrap-time contexts may run without one. Lifecycle From 4374434802109102388b83ae9a28b415ebecc5c3 Mon Sep 17 00:00:00 2001 From: Ralf Lang Date: Thu, 11 Jun 2026 23:11:04 +0200 Subject: [PATCH 4/5] feat(session): wire DefaultSessionSerializer in SessionHandlerFactory SessionHandlerFactory built SessionHandler with NativePhpSessionSerializer. That serializer wraps session_encode and session_decode, both of which require an active PHP session. Modern PSR-15 routes that own their session lifecycle through middleware never call session_start, so the native serializer cannot read or write rows for them. Switch to DefaultSessionSerializer (horde/sessionhandler). It dispatches to the pure-PHP PhpTextSessionSerializer for the default php session.serialize_handler and to PhpSessionSerializer for php_serialize. Both produce the exact byte sequence PHP's session module produces, so legacy callers that go through session_start continue to read and write the same on-disk rows. Modern and legacy stacks share storage. --- src/Factory/SessionHandlerFactory.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Factory/SessionHandlerFactory.php b/src/Factory/SessionHandlerFactory.php index c07877c0..84d106fe 100644 --- a/src/Factory/SessionHandlerFactory.php +++ b/src/Factory/SessionHandlerFactory.php @@ -20,7 +20,7 @@ use Horde\Core\Service\HordeDbService; use Horde\Core\Session\HordeSessionFactory; use Horde\HashTable\LockableHashTable; -use Horde\SessionHandler\NativePhpSessionSerializer; +use Horde\SessionHandler\DefaultSessionSerializer; use Horde\SessionHandler\SessionHandler; use Horde\SessionHandler\SessionStorageBackend; use Horde\SessionHandler\Storage\BuiltinBackend; @@ -90,7 +90,7 @@ public function create(Injector $injector): SessionHandler return new SessionHandler( backend: $backend, - serializer: new NativePhpSessionSerializer(), + serializer: new DefaultSessionSerializer(), sessionFactory: $this->createSessionFactory($injector), events: $this->getEventDispatcher($injector), ); From 9cf6fb9b74e5ec91b9ac5c67d75c1f861c745031 Mon Sep 17 00:00:00 2001 From: Ralf Lang Date: Thu, 11 Jun 2026 23:18:09 +0200 Subject: [PATCH 5/5] fixup! fix(session): type SessionLifecycle secret against an interface --- test/Unit/Session/SessionLifecycleTest.php | 37 +++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/test/Unit/Session/SessionLifecycleTest.php b/test/Unit/Session/SessionLifecycleTest.php index 8b036ce5..13f99d24 100644 --- a/test/Unit/Session/SessionLifecycleTest.php +++ b/test/Unit/Session/SessionLifecycleTest.php @@ -12,6 +12,7 @@ namespace Horde\Core\Test\Unit\Session; use Horde\Core\Config\State; +use Horde\Core\Secret\SessionSecret; use Horde\Core\Session\HordeSession; use Horde\Core\Session\SessionConfigFactory; use Horde\Core\Session\SessionLifecycle; @@ -55,6 +56,7 @@ class SessionLifecycleTest extends TestCase private function build( array $conf = [], ?HordeSession $session = null, + ?SessionSecret $secret = null, ): SessionLifecycle { $injector = new Injector(new TopLevel()); $session ??= new HordeSession(new SessionId('test-id'), []); @@ -63,7 +65,7 @@ private function build( $handler = new SessionHandler(new BuiltinBackend()); $config = (new SessionConfigFactory())->fromState(new State($conf)); - return new SessionLifecycle($injector, $handler, $config); + return new SessionLifecycle($injector, $handler, $config, $secret); } // --------------------------------------------------------------- @@ -83,6 +85,39 @@ public function testIsInactiveByDefault(): void self::assertFalse($this->build()->isActive()); } + #[Test] + public function testAcceptsSessionSecretImplementation(): void + { + // Regression sentinel for the binding-name-vs-class confusion. + // SessionLifecycle's secret arg is typed against the + // SessionSecret interface, not the legacy Horde_Secret_Cbc + // binding name. Anything that implements the interface + // (including production's Horde_Core_Secret_Cbc) must satisfy + // the constructor type check. + $secret = $this->createStub(SessionSecret::class); + $lifecycle = $this->build(secret: $secret); + self::assertInstanceOf(SessionLifecycle::class, $lifecycle); + } + + #[Test] + public function testAcceptsHordeCoreSecretCbcViaInterfaceContract(): void + { + // Belt-and-braces: the actual production class is + // Horde_Core_Secret_Cbc which now implements SessionSecret. + // The class is loadable in unit context (no DB / no session + // module needed for the constructor itself) so we can + // instantiate it without arguments and confirm it satisfies + // the lifecycle's type constraint. + if (!class_exists(\Horde_Core_Secret_Cbc::class)) { + self::markTestSkipped('Horde_Core_Secret_Cbc not loadable in this test environment'); + } + $secret = new \Horde_Core_Secret_Cbc(); + self::assertInstanceOf(SessionSecret::class, $secret); + + $lifecycle = $this->build(secret: $secret); + self::assertInstanceOf(SessionLifecycle::class, $lifecycle); + } + // --------------------------------------------------------------- // Cookie-domain guard // ---------------------------------------------------------------