Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion lib/Horde/Core/Secret/Cbc.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
* @package Core
*/

use Horde\Core\Secret\SessionSecret;
use Horde\Crypt\Blowfish\Blowfish;

/**
Expand All @@ -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.
Expand Down
2 changes: 2 additions & 0 deletions src/Auth/Jwt/JwtService.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
50 changes: 50 additions & 0 deletions src/Config/StateFactory.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
<?php

declare(strict_types=1);

/**
* Copyright 2026 The Horde Project (http://www.horde.org/)
*
* See the enclosed file LICENSE for license information (LGPL). If you
* did not receive this file, see http://www.horde.org/licenses/lgpl21.
*
* @category Horde
* @copyright 2026 The Horde Project
* @license http://www.horde.org/licenses/lgpl21 LGPL 2.1
* @package Core
*/

namespace Horde\Core\Config;

use Horde\Injector\Injector;

/**
* DI factory for {@see State}.
*
* Resolves the `horde` app's `conf.php` via {@see ConfigLoader}
* rather than depending on `$GLOBALS['conf']`. Modern PSR-15 routes
* that bypass `HordeCore` middleware do not get `$GLOBALS['conf']`
* populated; without this factory, autowired consumers of `State`
* (e.g. {@see LoggerConfig}) blow up with "Config neither passed
* nor available from global".
*
* The fallback in `State::__construct(?array $conf = null)` to
* `$GLOBALS['conf']` continues to work for legacy callers that
* construct `State` directly. The injector path goes through this
* factory and skips the fallback entirely.
*/
class StateFactory
{
public function create(Injector $injector): State
{
// Prefer a globally populated conf if it's already there:
// legacy stacks ran appInit() and wrote $GLOBALS['conf']
// before any DI lookup. Honour that to avoid double-loading.
if (isset($GLOBALS['conf']) && is_array($GLOBALS['conf'])) {
return new State($GLOBALS['conf']);
}

// Modern path: ConfigLoader reads conf.php directly from disk.
return $injector->getInstance(ConfigLoader::class)->load('horde');
}
}
3 changes: 3 additions & 0 deletions src/DefaultInjectorBindings.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions src/Factory/SessionHandlerFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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),
);
Expand Down
9 changes: 8 additions & 1 deletion src/Middleware/JwtSessionLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
) {}
Expand All @@ -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 === '') {
Expand Down
63 changes: 63 additions & 0 deletions src/Secret/SessionSecret.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
<?php

declare(strict_types=1);

/**
* Copyright 2026 The Horde Project (http://www.horde.org/)
*
* See the enclosed file LICENSE for license information (LGPL). If you
* did not receive this file, see http://www.horde.org/licenses/lgpl21.
*
* @category Horde
* @copyright 2026 The Horde Project
* @license http://www.horde.org/licenses/lgpl21 LGPL 2.1
* @package Core
*/

namespace Horde\Core\Secret;

/**
* Session-key cipher used by {@see \Horde\Core\Session\SessionLifecycle}
* to re-key encrypted session payloads on `clean()` / `destroy()`.
*
* Mirrors the two methods on legacy `Horde_Secret` that the lifecycle
* actually calls. Exists so DI consumers can name the contract by a
* type-hintable interface rather than the bare-string legacy binding
* `Horde_Secret_Cbc`. The legacy class hierarchy
* (`Horde_Core_Secret_Cbc extends Horde_Core_Secret extends Horde_Secret`)
* does not extend `Horde_Secret_Cbc`, so a `?Horde_Secret_Cbc`
* type hint rejects the real instance the injector returns.
*
* Implementations: {@see \Horde_Core_Secret_Cbc}.
*
* @internal Until other consumers materialise. The contract is
* stable but the namespace may move when the broader
* Secret stack modernises.
*/
interface SessionSecret
{
/**
* Set or rotate the per-session encryption key.
*
* Untyped on purpose: matches the legacy `Horde_Secret::setKey()`
* signature so existing implementations satisfy this interface
* without modification.
*
* @param string $keyname Logical name of the key to set.
* @return mixed Whatever the underlying implementation returns
* (legacy returns the generated key string).
*/
public function setKey($keyname = 'generic');

/**
* Clear the per-session encryption key.
*
* Untyped on purpose: matches the legacy
* `Horde_Secret::clearKey()` signature.
*
* @param string $keyname Logical name of the key to clear.
* @return mixed Whatever the underlying implementation returns
* (legacy returns bool).
*/
public function clearKey($keyname = 'generic');
}
12 changes: 6 additions & 6 deletions src/Session/SessionLifecycle.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@

namespace Horde\Core\Session;

use Horde\Core\Secret\SessionSecret;
use Horde\Injector\Attribute\Factory;
use Horde\Injector\Injector;
use Horde\SessionHandler\SessionHandler;
use Horde\SessionHandler\SessionId;
use Horde_Exception;
use Horde_Secret_Cbc;
use Horde_Shutdown;
use Horde_Shutdown_Task;

Expand All @@ -32,7 +32,7 @@
* {@see HordeSession} (the per-request data layer) plus Horde-application
* glue: PHP ini tuning, the cookie-domain/single-label-hostname guard, the
* shutdown task that mirrors {@see HordeSession} payload back into
* `$_SESSION`, and optional {@see Horde_Secret_Cbc} re-keying on `clean()` /
* `$_SESSION`, and optional {@see SessionSecret} re-keying on `clean()` /
* `destroy()`.
*
* Registry-agnostic. The legacy stack's relogin auth-changed guard lives
Expand Down Expand Up @@ -122,14 +122,14 @@ class SessionLifecycle implements Horde_Shutdown_Task
* {@see SessionConfigFactory} from
* the same `ConfigLoader` state
* {@see SessionHandlerFactory} uses.
* @param Horde_Secret_Cbc|null $secret Optional. Re-keyed on clean(),
* @param SessionSecret|null $secret Optional. Re-keyed on clean(),
* cleared on destroy().
*/
public function __construct(
private readonly Injector $injector,
private readonly SessionHandler $handler,
private readonly SessionConfig $config,
private readonly ?Horde_Secret_Cbc $secret = null,
private readonly ?SessionSecret $secret = null,
) {}

/**
Expand Down Expand Up @@ -256,7 +256,7 @@ public function close(): void
*
* Regenerates the session ID, clears all session data, rebuilds the
* modern session over the now-empty `$_SESSION`, writes fresh
* `_b`/`_r` timestamps, and rotates the {@see Horde_Secret_Cbc} key.
* `_b`/`_r` timestamps, and rotates the {@see SessionSecret} key.
* Idempotent: returns false on repeat calls within the same request.
*
* @return bool True if cleaned, false if already cleaned this request.
Expand Down Expand Up @@ -297,7 +297,7 @@ public function clean(): bool
/**
* Hard logout. Destroys the PHP session, clears `$_SESSION`, rebuilds
* the modern session over the empty payload, and clears the
* {@see Horde_Secret_Cbc} key.
* {@see SessionSecret} key.
*/
public function destroy(): void
{
Expand Down
25 changes: 17 additions & 8 deletions src/Session/SessionLifecycleFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,23 +16,26 @@

namespace Horde\Core\Session;

use Horde\Core\Secret\SessionSecret;
use Horde\Injector\Injector;
use Horde\SessionHandler\SessionHandler;
use Horde_Secret_Cbc;

/**
* DI factory for {@see SessionLifecycle}.
*
* Wires the modern {@see SessionHandler}, the {@see Injector} (used for
* lazy {@see HordeSession} resolution), the typed
* {@see SessionConfig} built by {@see SessionConfigFactory}, and the
* optional {@see Horde_Secret_Cbc} into a single per-request lifecycle
* orchestrator.
* optional {@see SessionSecret} cipher into a single per-request
* lifecycle orchestrator.
*
* The legacy binding name `Horde_Secret_Cbc` is used deliberately. Asking
* for the `Horde_Core_Secret_Cbc` class directly bypasses the binding and
* yields an instance with no IV configured. Matches what
* {@see HordeSessionFactory::create()} does.
* The legacy `Horde_Secret_Cbc` binding name is queried first because
* existing installs configure the cipher under that string. The
* resolved instance is checked against {@see SessionSecret} (which the
* real `Horde_Core_Secret_Cbc` implements) before being passed to the
* lifecycle: tests and minimal bootstraps may bind the legacy name to
* a fixture that does not implement the contract, in which case the
* lifecycle runs without re-keying.
*/
class SessionLifecycleFactory
{
Expand All @@ -50,7 +53,13 @@ public function create(Injector $injector): SessionLifecycle

$secret = null;
try {
$secret = $injector->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
Expand Down
37 changes: 36 additions & 1 deletion test/Unit/Session/SessionLifecycleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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'), []);
Expand All @@ -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);
}

// ---------------------------------------------------------------
Expand All @@ -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
// ---------------------------------------------------------------
Expand Down
Loading