diff --git a/src/Infrastructure/Adapter/In/Cli/Init.php b/src/Infrastructure/Adapter/In/Cli/Init.php index a2afb55e7..151ad79a9 100644 --- a/src/Infrastructure/Adapter/In/Cli/Init.php +++ b/src/Infrastructure/Adapter/In/Cli/Init.php @@ -86,6 +86,15 @@ public function initialize(string $controller): void // Load language $this->language->setLanguage(); + // Before initCli(), which is what runs the command. Without this the dispatcher has no + // receivers at all, and notify() is a loop over receivers — so every event a CLI command + // fired was silently discarded, including the ones that write the Eventlog table the + // security log reads. A master password rotation or a backup run from bin/cli.php left no + // trace of having happened, while the same operation through the web or the API was fully + // recorded. Web/Init and Api/Init both call this from their own initialize(); this door + // was the one that never had it. + $this->initEventHandlers(); + $this->initCli(); } diff --git a/tests/Integration/Infrastructure/Adapter/In/EventHandlersAreAttachedTest.php b/tests/Integration/Infrastructure/Adapter/In/EventHandlersAreAttachedTest.php new file mode 100644 index 000000000..64ac11ecb --- /dev/null +++ b/tests/Integration/Infrastructure/Adapter/In/EventHandlersAreAttachedTest.php @@ -0,0 +1,242 @@ +. + */ + +namespace SP\Tests\Integration\Infrastructure\Adapter\In; + +use DI\ContainerBuilder; +use PHPUnit\Framework\Attributes\DataProvider; +use PHPUnit\Framework\Attributes\Group; +use PHPUnit\Framework\Attributes\Test; +use PHPUnit\Framework\TestCase; +use Psr\Container\ContainerInterface; +use ReflectionMethod; +use SP\Domain\Core\Bootstrap\ModuleInterface; +use SP\Domain\Core\Bootstrap\Path; +use SP\Domain\Core\Events\EventDispatcherInterface; +use SP\Domain\File\FileSystem; +use SP\Infrastructure\Definitions\CoreDefinitions; +use SP\Infrastructure\Definitions\DomainDefinitions; +use SP\Infrastructure\ModuleBase; +use SP\Infrastructure\ProvidersHelper; +use SP\Tests\Support\Stubs\InertConsoleApplication; +use Symfony\Component\Console\Application as ConsoleApplication; + +use function DI\create; +use function SP\Tests\getResource; + +/** + * ModuleBase::initEventHandlers() is what attaches the four EventReceivers that make the event + * bus do anything at all: LogHandler always, and -- once the application is installed -- + * DatabaseHandler (the Eventlog table the web UI's security log reads), MailEvent and + * NotificationEvent. Nothing fires unless something is attached to receive it. + * + * Web\Init::initialize() calls it at line 223 and Api\Init::initialize() calls it at line 122. + * Cli\Init::initialize() never called it at all: sp:backup and sp:crypt:update-master-password + * left nothing in the Eventlog table, while the same operations through the web or API were fully + * recorded. tests/Integration/Infrastructure/Adapter/In/Cli/CliTestCase.php -- the harness the + * real end-to-end CLI command tests use -- resolves a Command class straight out of the container + * and runs it with CommandTester, which is exactly what let this slip through: none of those tests + * go anywhere near Cli\Init::initialize(), so nothing was ever in a position to notice the missing + * call. + * + * This builds each module's real container -- DomainDefinitions + CoreDefinitions($module) + that + * module's own module.php, the same way CliTestCase and AccountAccessTest do -- and checks the + * same thing for all three doors, so the invariant is stated once rather than only for the one + * that broke. + * + * The CLI door is exercised through its actual, unmodified initialize(): the only thing swapped + * out is the Console\Application binding, so initCli() does not go on to actually run a command. + * Web and Api are not run through their own initialize() here. Doing that for real would need + * materially more than the CLI does: Web binds Context to a real PHP session, which this project + * only exercises under #[RunClassInSeparateProcess] (see SessionTest) precisely because a real + * session started in the shared integration process is not safe for the tests around it. Api's + * checkInstalled() runs before initEventHandlers(), so reaching it for real needs a config that + * claims to be installed, an app/database version that matches exactly (or checkUpgradeNeeded() + * intervenes first) and live checkDatabaseConnection()/checkDatabaseTables() checks against the + * real schema. Instead, the shared, protected method is invoked directly, by reflection, on a + * real container-built Init -- proving the ProvidersHelper/EventDispatcher wiring those two doors + * already depend on is sound, without paying for either door's full request lifecycle. + */ +#[Group('integration')] +final class EventHandlersAreAttachedTest extends TestCase +{ + private string $root; + private string $configPath; + + protected function setUp(): void + { + parent::setUp(); + + // A real (non-vfs), uniquely-named directory -- vfs is shared process-wide, and every + // test here builds its own container against its own config. + $this->root = FileSystem::buildPath( + sys_get_temp_dir(), + 'syspass-event-handlers-' . bin2hex(random_bytes(6)) + ); + $this->configPath = FileSystem::buildPath($this->root, 'config'); + + foreach ([$this->configPath, $this->cachePath(), $this->tmpPath(), $this->backupPath()] as $dir) { + if (!mkdir($dir, 0777, true) && !is_dir($dir)) { + self::fail(sprintf('Directory "%s" was not created', $dir)); + } + } + + // The same not-installed fixture config CliTestCase/AccountAccessTest use. + // ModuleBase::initEventHandlers() attaches only the LogHandler while the application is + // not installed, which is exactly what makes the LogHandler the one thing safe to assert + // on every door regardless of whether it also reaches DatabaseHandler/MailEvent/ + // NotificationEvent. + file_put_contents( + FileSystem::buildPath($this->configPath, 'config.xml'), + getResource('config', 'config.xml') + ); + } + + protected function tearDown(): void + { + FileSystem::rmdirRecursive($this->root); + + parent::tearDown(); + } + + /** + * The fix: Cli\Init::initialize() attaches the event handlers before initCli() hands control + * to the console application, exactly like the web and API doors already do. + */ + #[Test] + public function theCliEntryPointAttachesTheLogHandlerBeforeRunningAnyCommand(): void + { + $dic = $this->buildContainer('cli'); + + $dic->get(ModuleInterface::class)->initialize(''); + + self::assertHandlerAttached($dic, 'cli'); + } + + /** + * @return array + */ + public static function httpModuleProvider(): array + { + return ['web' => ['web'], 'api' => ['api']]; + } + + /** + * The invariant Cli\Init was missing, stated for the two doors that already had it -- see the + * class docblock for why these two go through initEventHandlers() directly by reflection + * rather than through their own initialize(). + */ + #[Test] + #[DataProvider('httpModuleProvider')] + public function theHttpEntryPointsAlreadyAttachTheLogHandler(string $module): void + { + $dic = $this->buildContainer($module); + $init = $dic->get(ModuleInterface::class); + + (new ReflectionMethod(ModuleBase::class, 'initEventHandlers'))->invoke($init); + + self::assertHandlerAttached($dic, $module); + } + + private static function assertHandlerAttached(ContainerInterface $dic, string $module): void + { + $dispatcher = $dic->get(EventDispatcherInterface::class); + $logHandler = $dic->get(ProvidersHelper::class)->getLogHandler(); + + self::assertTrue( + $dispatcher->has($logHandler), + sprintf( + 'The "%s" module\'s EventDispatcher has no LogHandler attached after ' + . 'initEventHandlers() ran, so nothing that module fires would ever reach the log ' + . 'or, once installed, the Eventlog table.', + $module + ) + ); + } + + private function buildContainer(string $module): ContainerInterface + { + // CoreDefinitions computes the paths when called: point the config (and with it the log + // file) at this test's own directory rather than the working copy's var/ state. + $_ENV['CONFIG_PATH'] = $this->configPath; + + try { + $coreDefinitions = CoreDefinitions::getDefinitions(REAL_APP_ROOT, $module); + } finally { + unset($_ENV['CONFIG_PATH']); + } + + $coreDefinitions['paths'] = array_map( + fn(array $path) => match ($path[0]) { + Path::CACHE => [Path::CACHE, $this->cachePath()], + Path::TMP => [Path::TMP, $this->tmpPath()], + Path::BACKUP => [Path::BACKUP, $this->backupPath()], + default => $path, + }, + $coreDefinitions['paths'] + ); + + $moduleDefinitions = FileSystem::require( + FileSystem::buildPath( + REAL_APP_ROOT, + 'src', + 'Infrastructure', + 'Adapter', + 'In', + ucfirst($module), + 'module.php' + ) + ); + + $builder = new ContainerBuilder(); + $definitions = [DomainDefinitions::getDefinitions(), $coreDefinitions, $moduleDefinitions]; + + if ($module === 'cli') { + // initCli() would otherwise hand off to the real console application and run whatever + // command was on the (empty) command line -- swap in a double whose run() does nothing. + $definitions[] = [ConsoleApplication::class => create(InertConsoleApplication::class)]; + } + + $builder->addDefinitions(...$definitions); + + return $builder->build(); + } + + private function cachePath(): string + { + return FileSystem::buildPath($this->root, 'cache'); + } + + private function tmpPath(): string + { + return FileSystem::buildPath($this->root, 'tmp'); + } + + private function backupPath(): string + { + return FileSystem::buildPath($this->root, 'backup'); + } +} diff --git a/tests/Support/Stubs/InertConsoleApplication.php b/tests/Support/Stubs/InertConsoleApplication.php new file mode 100644 index 000000000..0a8ceff1c --- /dev/null +++ b/tests/Support/Stubs/InertConsoleApplication.php @@ -0,0 +1,49 @@ +. + */ + +declare(strict_types=1); + +namespace SP\Tests\Support\Stubs; + +use Symfony\Component\Console\Application; +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Output\OutputInterface; + +/** + * A Console\Application whose run() does nothing. + * + * SP\Infrastructure\Adapter\In\Cli\Init::initCli() -- called from initialize(), after event + * handlers are attached -- ends by dispatching whatever command the CLI was invoked with. A test + * that wants to exercise initialize() itself, without actually running a command (and whatever + * that command does to the database or filesystem), swaps this in for the real Application via + * the container. run() is not final on the real class, so overriding it here is enough to make + * the rest of Init::initialize() safe to call directly. + */ +final class InertConsoleApplication extends Application +{ + public function run(?InputInterface $input = null, ?OutputInterface $output = null): int + { + return 0; + } +}