fix/jwt service factory binding#151
Merged
Merged
Conversation
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.
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.
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.
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.
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ties in to: horde/base#105 horde/SessionHandler#12
Fixes gaps in the modern JWT and Cookie based session stack