From bbf8c4c15b2ff54708d9d61471acebc583c42542 Mon Sep 17 00:00:00 2001 From: blaipr Date: Thu, 3 Sep 2026 03:16:30 +0200 Subject: [PATCH] fix: keep the configuration a save replaces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ConfigBackupService::backup() has existed since this rewrite was imported and is called from nowhere in src/, so config_backup was never written and the 'Download config backup' link the Information page renders — beside a date, unconditionally — answered 'Unable to retrieve the configuration' every time, on every installation. Every integration test around that download seeds the parameter by hand with #[InjectConfigParam], which is the shape of a feature nothing produces. On paper the call belongs in ConfigFile::save(). It cannot go there: ConfigBackup needs ConfigService, which needs Application, which needs ConfigFileService — a cycle only a lazy proxy breaks, on the object the container builds while booting. ConfigTrait::saveConfig() is the shared path all nine configuration screens go through, runs per-request long after boot, and a controller taking ConfigBackupService is already proven by DownloadConfigBackupController. Required rather than optional, because an optional collaborator a caller forgets is silently no backup — the failure being fixed. getConfigData() answers a clone and save() has not run when the backup is taken, so what is stored is the previous configuration. backup() logs and swallows its own failures, so a database that cannot take it does not stop an administrator saving. --- .../ConfigAccount/SaveController.php | 13 + .../Controllers/ConfigAuth/SaveController.php | 13 + .../ConfigDokuWiki/SaveController.php | 13 + .../ConfigEvents/SaveController.php | 13 + .../ConfigGeneral/SaveController.php | 5 +- .../Controllers/ConfigLdap/SaveController.php | 13 + .../Controllers/ConfigMail/SaveController.php | 13 + .../ConfigSecurity/SaveController.php | 3 + .../Controllers/ConfigWiki/SaveController.php | 13 + .../In/Web/Controllers/Traits/ConfigTrait.php | 25 +- .../ConfigAccount/RefusalsTest.php | 7 +- .../ConfigAuth/ConfigBackupOnSaveTest.php | 260 ++++++++++++++++++ .../Controllers/ConfigAuth/RefusalsTest.php | 7 +- .../Controllers/ConfigEvents/RefusalsTest.php | 7 +- .../ConfigGeneral/RefusalsTest.php | 6 +- .../Controllers/ConfigLdap/RefusalsTest.php | 15 +- .../Controllers/ConfigMail/RefusalsTest.php | 7 +- .../Controllers/ConfigWiki/RefusalsTest.php | 7 +- 18 files changed, 421 insertions(+), 19 deletions(-) create mode 100644 tests/Unit/Infrastructure/Adapter/In/Web/Controllers/ConfigAuth/ConfigBackupOnSaveTest.php diff --git a/src/Infrastructure/Adapter/In/Web/Controllers/ConfigAccount/SaveController.php b/src/Infrastructure/Adapter/In/Web/Controllers/ConfigAccount/SaveController.php index 24b9a8234..0bf3430aa 100644 --- a/src/Infrastructure/Adapter/In/Web/Controllers/ConfigAccount/SaveController.php +++ b/src/Infrastructure/Adapter/In/Web/Controllers/ConfigAccount/SaveController.php @@ -24,6 +24,9 @@ namespace SP\Infrastructure\Adapter\In\Web\Controllers\ConfigAccount; +use SP\Infrastructure\Adapter\In\Web\Controllers\Helpers\SimpleControllerHelper; +use SP\Application\Application; +use SP\Application\Config\Ports\ConfigBackupService; use SP\Domain\Core\Events\Event; use SP\Domain\Core\Events\EventMessage; use SP\Domain\Common\Attributes\Action; @@ -44,8 +47,17 @@ */ final class SaveController extends SimpleControllerBase { + use ConfigTrait; + public function __construct( + Application $application, + SimpleControllerHelper $simpleControllerHelper, + private readonly ConfigBackupService $configBackup + ) { + parent::__construct($application, $simpleControllerHelper); + } + private const MAX_FILES_SIZE = 16384; private const SECONDS_PER_DAY = 24 * 3600; @@ -76,6 +88,7 @@ public function saveAction(): ActionResponse return $this->saveConfig( $configData, $this->config, + $this->configBackup, fn() => $this->eventDispatcher->notify(new Event('save.config.account', $this, $eventMessage)) ); } diff --git a/src/Infrastructure/Adapter/In/Web/Controllers/ConfigAuth/SaveController.php b/src/Infrastructure/Adapter/In/Web/Controllers/ConfigAuth/SaveController.php index 4cd159687..8f10c44ba 100644 --- a/src/Infrastructure/Adapter/In/Web/Controllers/ConfigAuth/SaveController.php +++ b/src/Infrastructure/Adapter/In/Web/Controllers/ConfigAuth/SaveController.php @@ -3,6 +3,9 @@ namespace SP\Infrastructure\Adapter\In\Web\Controllers\ConfigAuth; +use SP\Infrastructure\Adapter\In\Web\Controllers\Helpers\SimpleControllerHelper; +use SP\Application\Application; +use SP\Application\Config\Ports\ConfigBackupService; use SP\Domain\Core\Events\Event; use SP\Domain\Core\Events\EventMessage; use SP\Domain\Common\Attributes\Action; @@ -18,8 +21,17 @@ final class SaveController extends SimpleControllerBase { + use ConfigTrait; + public function __construct( + Application $application, + SimpleControllerHelper $simpleControllerHelper, + private readonly ConfigBackupService $configBackup + ) { + parent::__construct($application, $simpleControllerHelper); + } + #[Action(ResponseType::JSON)] public function saveAction(): ActionResponse { @@ -31,6 +43,7 @@ public function saveAction(): ActionResponse return $this->saveConfig( $configData, $this->config, + $this->configBackup, fn() => $this->eventDispatcher->notify(new Event('save.config.auth', $this, $eventMessage)) ); } diff --git a/src/Infrastructure/Adapter/In/Web/Controllers/ConfigDokuWiki/SaveController.php b/src/Infrastructure/Adapter/In/Web/Controllers/ConfigDokuWiki/SaveController.php index fd86dfcc2..502ed4466 100644 --- a/src/Infrastructure/Adapter/In/Web/Controllers/ConfigDokuWiki/SaveController.php +++ b/src/Infrastructure/Adapter/In/Web/Controllers/ConfigDokuWiki/SaveController.php @@ -24,6 +24,9 @@ namespace SP\Infrastructure\Adapter\In\Web\Controllers\ConfigDokuWiki; +use SP\Infrastructure\Adapter\In\Web\Controllers\Helpers\SimpleControllerHelper; +use SP\Application\Application; +use SP\Application\Config\Ports\ConfigBackupService; use SP\Domain\Core\Events\Event; use SP\Domain\Core\Events\EventMessage; use SP\Domain\Common\Attributes\Action; @@ -38,8 +41,17 @@ final class SaveController extends SimpleControllerBase { + use ConfigTrait; + public function __construct( + Application $application, + SimpleControllerHelper $simpleControllerHelper, + private readonly ConfigBackupService $configBackup + ) { + parent::__construct($application, $simpleControllerHelper); + } + #[Action(ResponseType::JSON)] public function saveAction(): ActionResponse { @@ -79,6 +91,7 @@ public function saveAction(): ActionResponse return $this->saveConfig( $configData, $this->config, + $this->configBackup, function () use ($eventMessage) { $this->eventDispatcher->notify(new Event('save.config.dokuwiki', $this, $eventMessage)); } diff --git a/src/Infrastructure/Adapter/In/Web/Controllers/ConfigEvents/SaveController.php b/src/Infrastructure/Adapter/In/Web/Controllers/ConfigEvents/SaveController.php index c82e998e3..be4b0bfe7 100644 --- a/src/Infrastructure/Adapter/In/Web/Controllers/ConfigEvents/SaveController.php +++ b/src/Infrastructure/Adapter/In/Web/Controllers/ConfigEvents/SaveController.php @@ -3,6 +3,9 @@ namespace SP\Infrastructure\Adapter\In\Web\Controllers\ConfigEvents; +use SP\Infrastructure\Adapter\In\Web\Controllers\Helpers\SimpleControllerHelper; +use SP\Application\Application; +use SP\Application\Config\Ports\ConfigBackupService; use SP\Domain\Core\Events\Event; use SP\Domain\Core\Events\EventMessage; use SP\Domain\Common\Attributes\Action; @@ -20,8 +23,17 @@ final class SaveController extends SimpleControllerBase { + use ConfigTrait; + public function __construct( + Application $application, + SimpleControllerHelper $simpleControllerHelper, + private readonly ConfigBackupService $configBackup + ) { + parent::__construct($application, $simpleControllerHelper); + } + #[Action(ResponseType::JSON)] public function saveAction(): ActionResponse { @@ -33,6 +45,7 @@ public function saveAction(): ActionResponse return $this->saveConfig( $configData, $this->config, + $this->configBackup, fn() => $this->eventDispatcher->notify(new Event('save.config.events', $this, $eventMessage)) ); } diff --git a/src/Infrastructure/Adapter/In/Web/Controllers/ConfigGeneral/SaveController.php b/src/Infrastructure/Adapter/In/Web/Controllers/ConfigGeneral/SaveController.php index 9f6375a11..371f91d51 100644 --- a/src/Infrastructure/Adapter/In/Web/Controllers/ConfigGeneral/SaveController.php +++ b/src/Infrastructure/Adapter/In/Web/Controllers/ConfigGeneral/SaveController.php @@ -24,6 +24,7 @@ namespace SP\Infrastructure\Adapter\In\Web\Controllers\ConfigGeneral; +use SP\Application\Config\Ports\ConfigBackupService; use SP\Application\Application; use SP\Domain\Core\Events\Event; use SP\Domain\Common\Attributes\Action; @@ -53,7 +54,8 @@ final class SaveController extends SimpleControllerBase public function __construct( Application $application, SimpleControllerHelper $simpleControllerHelper, - private readonly AppLockHandler $appLock + private readonly AppLockHandler $appLock, + private readonly ConfigBackupService $configBackup ) { parent::__construct($application, $simpleControllerHelper); } @@ -74,6 +76,7 @@ public function saveAction(): ActionResponse return $this->saveConfig( $configData, $this->config, + $this->configBackup, function () use ($configData) { if ($configData->isMaintenance()) { $this->appLock->lock($this->session->getUserData()->id, 'config'); diff --git a/src/Infrastructure/Adapter/In/Web/Controllers/ConfigLdap/SaveController.php b/src/Infrastructure/Adapter/In/Web/Controllers/ConfigLdap/SaveController.php index 7b0cfae8b..558b6937e 100644 --- a/src/Infrastructure/Adapter/In/Web/Controllers/ConfigLdap/SaveController.php +++ b/src/Infrastructure/Adapter/In/Web/Controllers/ConfigLdap/SaveController.php @@ -24,6 +24,9 @@ namespace SP\Infrastructure\Adapter\In\Web\Controllers\ConfigLdap; +use SP\Infrastructure\Adapter\In\Web\Controllers\Helpers\SimpleControllerHelper; +use SP\Application\Application; +use SP\Application\Config\Ports\ConfigBackupService; use SP\Domain\Core\Events\Event; use SP\Domain\Core\Events\EventMessage; use SP\Domain\Auth\Providers\Ldap\LdapParams; @@ -47,8 +50,17 @@ */ final class SaveController extends SimpleControllerBase { + use ConfigTrait; + public function __construct( + Application $application, + SimpleControllerHelper $simpleControllerHelper, + private readonly ConfigBackupService $configBackup + ) { + parent::__construct($application, $simpleControllerHelper); + } + /** * @throws ValidationException * @throws SPException @@ -116,6 +128,7 @@ public function saveAction(): ActionResponse return $this->saveConfig( $configData, $this->config, + $this->configBackup, function () use ($eventMessage) { $this->eventDispatcher->notify(new Event('save.config.ldap', $this, $eventMessage)); } diff --git a/src/Infrastructure/Adapter/In/Web/Controllers/ConfigMail/SaveController.php b/src/Infrastructure/Adapter/In/Web/Controllers/ConfigMail/SaveController.php index 72964a0a6..06ffe6c2c 100644 --- a/src/Infrastructure/Adapter/In/Web/Controllers/ConfigMail/SaveController.php +++ b/src/Infrastructure/Adapter/In/Web/Controllers/ConfigMail/SaveController.php @@ -24,6 +24,9 @@ namespace SP\Infrastructure\Adapter\In\Web\Controllers\ConfigMail; +use SP\Infrastructure\Adapter\In\Web\Controllers\Helpers\SimpleControllerHelper; +use SP\Application\Application; +use SP\Application\Config\Ports\ConfigBackupService; use SP\Domain\Core\Events\Event; use SP\Domain\Core\Events\EventMessage; use SP\Domain\Common\Attributes\Action; @@ -44,8 +47,17 @@ */ final class SaveController extends SimpleControllerBase { + use ConfigTrait; + public function __construct( + Application $application, + SimpleControllerHelper $simpleControllerHelper, + private readonly ConfigBackupService $configBackup + ) { + parent::__construct($application, $simpleControllerHelper); + } + /** * @throws SPException */ @@ -108,6 +120,7 @@ public function saveAction(): ActionResponse return $this->saveConfig( $configData, $this->config, + $this->configBackup, function () use ($eventMessage) { $this->eventDispatcher->notify(new Event('save.config.mail', $this, $eventMessage)); } diff --git a/src/Infrastructure/Adapter/In/Web/Controllers/ConfigSecurity/SaveController.php b/src/Infrastructure/Adapter/In/Web/Controllers/ConfigSecurity/SaveController.php index 9e8a0ec78..54520bc38 100644 --- a/src/Infrastructure/Adapter/In/Web/Controllers/ConfigSecurity/SaveController.php +++ b/src/Infrastructure/Adapter/In/Web/Controllers/ConfigSecurity/SaveController.php @@ -24,6 +24,7 @@ namespace SP\Infrastructure\Adapter\In\Web\Controllers\ConfigSecurity; +use SP\Application\Config\Ports\ConfigBackupService; use SP\Application\Application; use SP\Domain\Core\Events\Event; use SP\Domain\Common\Attributes\Action; @@ -43,6 +44,7 @@ final class SaveController extends SimpleControllerBase public function __construct( Application $application, SimpleControllerHelper $simpleControllerHelper, + private readonly ConfigBackupService $configBackup ) { parent::__construct($application, $simpleControllerHelper); } @@ -61,6 +63,7 @@ public function saveAction(): ActionResponse return $this->saveConfig( $configData, $this->config, + $this->configBackup, function () { $this->eventDispatcher->notify(new Event('save.config.security', $this)); } diff --git a/src/Infrastructure/Adapter/In/Web/Controllers/ConfigWiki/SaveController.php b/src/Infrastructure/Adapter/In/Web/Controllers/ConfigWiki/SaveController.php index ec4339258..e7f4c78f2 100644 --- a/src/Infrastructure/Adapter/In/Web/Controllers/ConfigWiki/SaveController.php +++ b/src/Infrastructure/Adapter/In/Web/Controllers/ConfigWiki/SaveController.php @@ -24,6 +24,9 @@ namespace SP\Infrastructure\Adapter\In\Web\Controllers\ConfigWiki; +use SP\Infrastructure\Adapter\In\Web\Controllers\Helpers\SimpleControllerHelper; +use SP\Application\Application; +use SP\Application\Config\Ports\ConfigBackupService; use JsonException; use SP\Domain\Core\Events\Event; use SP\Domain\Core\Events\EventMessage; @@ -43,8 +46,17 @@ */ final class SaveController extends SimpleControllerBase { + use ConfigTrait; + public function __construct( + Application $application, + SimpleControllerHelper $simpleControllerHelper, + private readonly ConfigBackupService $configBackup + ) { + parent::__construct($application, $simpleControllerHelper); + } + /** * @return ActionResponse @@ -85,6 +97,7 @@ public function saveAction(): ActionResponse return $this->saveConfig( $configData, $this->config, + $this->configBackup, function () use ($eventMessage) { $this->eventDispatcher->notify(new Event('save.config.wiki', $this, $eventMessage)); } diff --git a/src/Infrastructure/Adapter/In/Web/Controllers/Traits/ConfigTrait.php b/src/Infrastructure/Adapter/In/Web/Controllers/Traits/ConfigTrait.php index 12109fade..5db5cd0a5 100644 --- a/src/Infrastructure/Adapter/In/Web/Controllers/Traits/ConfigTrait.php +++ b/src/Infrastructure/Adapter/In/Web/Controllers/Traits/ConfigTrait.php @@ -27,6 +27,7 @@ use Exception; use SP\Domain\Common\Dtos\ActionResponse; use SP\Domain\Config\Ports\ConfigDataInterface; +use SP\Application\Config\Ports\ConfigBackupService; use SP\Application\Config\Ports\ConfigFileService; use SP\Domain\Core\Exceptions\SPException; @@ -47,14 +48,34 @@ trait ConfigTrait */ protected function saveConfig( ConfigDataInterface $configData, - ConfigFileService $config, - ?callable $onSuccess = null + ConfigFileService $config, + ConfigBackupService $configBackup, + ?callable $onSuccess = null ): ActionResponse { try { if ($configData->isDemoEnabled()) { return ActionResponse::warning(__u('Ey, this is a DEMO!!')); } + // Keep the configuration being replaced, so there is something to go back to. + // + // `ConfigBackupService::backup()` has existed since this rewrite was imported and was + // called from nowhere, so `config_backup` was never written — and the "Download config + // backup" link the Information page renders answered "Unable to retrieve the + // configuration" every time, for every installation. + // + // Here rather than inside `ConfigFile::save()`, which is where it belongs on paper: + // that would put `ConfigBackupService` in the constructor of a service the container + // builds while booting, and it needs `ConfigService`, which needs `Application`, which + // needs `ConfigFileService` — a cycle that only a lazy proxy breaks, on the one object + // every request depends on. This is every door an administrator changes configuration + // through, and it costs the boot path nothing. + // + // `getConfigData()` answers a clone, and `save()` has not run yet, so what it hands + // over here is still the previous configuration. `backup()` logs and swallows its own + // failures, so a database that cannot take it does not stop the save. + $configBackup->backup($config->getConfigData()); + $config->save($configData); if ($onSuccess !== null) { diff --git a/tests/Unit/Infrastructure/Adapter/In/Web/Controllers/ConfigAccount/RefusalsTest.php b/tests/Unit/Infrastructure/Adapter/In/Web/Controllers/ConfigAccount/RefusalsTest.php index 2376361cf..7f1618114 100644 --- a/tests/Unit/Infrastructure/Adapter/In/Web/Controllers/ConfigAccount/RefusalsTest.php +++ b/tests/Unit/Infrastructure/Adapter/In/Web/Controllers/ConfigAccount/RefusalsTest.php @@ -27,6 +27,7 @@ namespace SP\Tests\Unit\Infrastructure\Adapter\In\Web\Controllers\ConfigAccount; +use SP\Application\Config\Ports\ConfigBackupService; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\Exception; @@ -93,7 +94,8 @@ public function savingIsRefusedWhenTheAclDenies(): void new SaveController( $application, - $this->simpleControllerHelper($acl, 'configAccount', 'save') + $this->simpleControllerHelper($acl, 'configAccount', 'save'), + self::createStub(ConfigBackupService::class) ); } @@ -116,7 +118,8 @@ public function savingReportsAFailureBehindItRatherThanEscaping(): void $response = (new SaveController( $application, - $this->simpleControllerHelper($acl, 'configAccount', 'save') + $this->simpleControllerHelper($acl, 'configAccount', 'save'), + self::createStub(ConfigBackupService::class) ))->saveAction(); self::assertSame(ResponseStatus::ERROR, $response->status); diff --git a/tests/Unit/Infrastructure/Adapter/In/Web/Controllers/ConfigAuth/ConfigBackupOnSaveTest.php b/tests/Unit/Infrastructure/Adapter/In/Web/Controllers/ConfigAuth/ConfigBackupOnSaveTest.php new file mode 100644 index 000000000..e9e18ba73 --- /dev/null +++ b/tests/Unit/Infrastructure/Adapter/In/Web/Controllers/ConfigAuth/ConfigBackupOnSaveTest.php @@ -0,0 +1,260 @@ +. + */ + +namespace SP\Tests\Unit\Infrastructure\Adapter\In\Web\Controllers\ConfigAuth; + +use SP\Application\Config\Ports\ConfigBackupService; +use PHPUnit\Framework\Attributes\Group; +use PHPUnit\Framework\Attributes\Test; +use PHPUnit\Framework\MockObject\Exception; +use SP\Application\Application; +use SP\Application\Config\Ports\ConfigFileService; +use SP\Domain\Config\Ports\ConfigDataInterface; +use SP\Domain\Core\Acl\AclInterface; +use SP\Domain\Core\Acl\UnauthorizedPageException; +use SP\Domain\Core\Bootstrap\RouteContextData; +use SP\Domain\Core\Bootstrap\UriContextInterface; +use SP\Domain\Core\Context\SessionContext; +use SP\Domain\Core\UI\ThemeInterface; +use SP\Domain\Common\Enums\ResponseStatus; +use SP\Domain\Http\Ports\RequestService; +use SP\Domain\User\Dtos\UserDto; +use SP\Domain\User\Models\ProfileData; +use SP\Infrastructure\Adapter\In\Web\Controllers\ConfigAuth\SaveController; +use SP\Infrastructure\Adapter\In\Web\Controllers\Helpers\SimpleControllerHelper; +use SP\Infrastructure\Bootstrap\Router; +use SP\Infrastructure\Events\EventDispatcher; +use SP\Infrastructure\Http\Ports\ResponseService; +use SP\Infrastructure\PhpExtensionChecker; +use SP\Tests\Support\WebControllerTestCase; +use Symfony\Component\HttpFoundation\Request; + +/** + * What this action does when the ACL says no. + * + * `ConfigAuth` has a single controller, and it — like every other controller in the `Config*` + * families — extends `SimpleControllerBase` rather than `ControllerBase`, checking access from + * `initialize()`, which the base constructor calls. So the refusal here is not an `ActionResponse` + * an action returns, it is an `UnauthorizedPageException` thrown while *building* the controller. + * Reaching it needs a `SimpleControllerHelper` rather than the `WebControllerHelper` the shared + * harness builds, so `simpleControllerHelper()` below assembles one the same way the harness + * assembles its own (see `WebControllerTestCase::webControllerHelper()`). + * + * `applicationForASignedInUser()` cannot be reused as-is either: `SimpleControllerBase:: + * $eventDispatcher` is typed to the concrete, `final` `EventDispatcher`, not + * `EventDispatcherInterface`, and that assignment runs before `initialize()` ever gets to check the + * ACL — so a stub of the interface is a property-type `TypeError` on construction, refusal or not. + * `signedInUserApplication()` mirrors that helper with a real `EventDispatcher` instead. + * + * `SaveController` here takes no collaborator of its own — only the `Application` and helper every + * `SimpleControllerBase` needs — so there is nothing to assert was "never called" beyond the + * exception itself. + * + * `saveAction()` delegates to `ConfigTrait::saveConfig()`, which does carry a `catch` — around + * `ConfigFileService::save()` — so that gets a test for the failure path too. + */ +/** + * Saving configuration keeps the configuration it replaced. + * + * `ConfigBackupService::backup()` has existed since this rewrite was imported and was called from + * nowhere, so `config_backup` was never written — and the "Download config backup" link the + * Information page renders answered "Unable to retrieve the configuration" every time, on every + * installation. The integration tests around that download all seed the parameter by hand with + * `#[InjectConfigParam]`, which is the shape of a feature nothing produces. + * + * The backup is taken in `ConfigTrait::saveConfig()`, which every one of the nine configuration + * screens goes through, so one controller stands for all of them here. + */ +#[Group('unitary')] +class ConfigBackupOnSaveTest extends WebControllerTestCase +{ + /** + * @throws Exception + */ + #[Test] + public function savingKeepsTheConfigurationItReplaces(): void + { + $configBackup = $this->createMock(ConfigBackupService::class); + $configBackup->expects(self::once())->method('backup'); + + (new SaveController( + $this->signedInUserApplication(), + $this->simpleControllerHelper($this->aclThatAllows(), 'configAuth', 'save'), + $configBackup + ))->saveAction(); + } + + /** + * And takes it *before* the save, or it would keep the configuration it was replacing it with. + * + * `ConfigFileService::getConfigData()` answers a clone and `save()` has not run yet, so what + * reaches `backup()` is the stored configuration rather than the one being written. Ordering is + * the only thing that makes that true, so it is asserted directly. + * + * @throws Exception + */ + #[Test] + public function theBackupIsTakenBeforeTheNewConfigurationIsWritten(): void + { + $order = []; + + $config = $this->createStub(ConfigFileService::class); + $config->method('getConfigData')->willReturn($this->demoDisabledConfigData()); + $config->method('save')->willReturnCallback( + function () use (&$order, &$config): ConfigFileService { + $order[] = 'save'; + + return $config; + } + ); + + $configBackup = $this->createStub(ConfigBackupService::class); + $configBackup->method('backup')->willReturnCallback( + static function () use (&$order): void { + $order[] = 'backup'; + } + ); + + (new SaveController( + new Application($config, new EventDispatcher(), $this->signedInUserSession()), + $this->simpleControllerHelper($this->aclThatAllows(), 'configAuth', 'save'), + $configBackup + ))->saveAction(); + + self::assertSame(['backup', 'save'], $order); + } + + /** + * A demo instance refuses the save, and stores no backup of a configuration it did not replace. + * + * @throws Exception + */ + #[Test] + public function aRefusedSaveKeepsNothing(): void + { + $demoConfigData = $this->createStub(ConfigDataInterface::class); + $demoConfigData->method('isDemoEnabled')->willReturn(true); + $demoConfigData->method('getPasswordSalt')->willReturn('the-password-salt'); + + $config = $this->createStub(ConfigFileService::class); + $config->method('getConfigData')->willReturn($demoConfigData); + + $configBackup = $this->createMock(ConfigBackupService::class); + $configBackup->expects(self::never())->method('backup'); + + $response = (new SaveController( + new Application($config, new EventDispatcher(), $this->signedInUserSession()), + $this->simpleControllerHelper($this->aclThatAllows(), 'configAuth', 'save'), + $configBackup + ))->saveAction(); + + self::assertSame(ResponseStatus::WARNING, $response->status); + } + + /** + * `SimpleControllerBase` takes a `SimpleControllerHelper`, not the `WebControllerHelper` the + * shared harness builds for `ControllerBase` subclasses — this mirrors + * `WebControllerTestCase::webControllerHelper()`'s construction of that inner object directly. + * + * @throws Exception + */ + private function simpleControllerHelper( + AclInterface $acl, + string $controller = 'controller', + string $action = 'action' + ): SimpleControllerHelper { + $request = $this->createStub(RequestService::class); + $request->method('isAjax')->willReturn(false); + $request->method('getServer')->willReturn('0'); + $request->method('analyzeString')->willReturn(null); + $request->method('analyzeArray')->willReturn(null); + $request->method('analyzeInt')->willReturn(null); + + $theme = $this->createStub(ThemeInterface::class); + $theme->method('getUri')->willReturn('/theme'); + + $uriContext = $this->createStub(UriContextInterface::class); + $uriContext->method('getWebRoot')->willReturn('https://syspass.invalid'); + $uriContext->method('getSubUri')->willReturn('/index.php'); + + return new SimpleControllerHelper( + $theme, + new Router(new Request(), $this->createStub(ResponseService::class)), + $acl, + $request, + new PhpExtensionChecker(), + $uriContext, + new RouteContextData($controller, $action, $action . 'Action', []) + ); + } + /** + * Mirrors `WebControllerTestCase::applicationForASignedInUser()`, with a real `EventDispatcher` + * (see the class docblock) in place of a stubbed `EventDispatcherInterface`. + * + * @throws Exception + */ + private function signedInUserApplication(): Application + { + $config = $this->createStub(ConfigFileService::class); + $config->method('getConfigData')->willReturn($this->demoDisabledConfigData()); + + return new Application($config, new EventDispatcher(), $this->signedInUserSession()); + } + /** + * @throws Exception + */ + private function signedInUserSession(): SessionContext + { + $session = $this->createStub(SessionContext::class); + $session->method('isLoggedIn')->willReturn(true); + $session->method('getAuthCompleted')->willReturn(true); + $session->method('getUserData')->willReturn( + new UserDto( + id: 7, + userGroupId: 2, + login: 'jdoe', + ssoLogin: 'jdoe@sso.example', + isAdminApp: false, + isAdminAcc: false + ) + ); + $session->method('getUserProfile')->willReturn(new ProfileData()); + + return $session; + } + /** + * @throws Exception + */ + private function demoDisabledConfigData(): ConfigDataInterface + { + $configData = $this->createStub(ConfigDataInterface::class); + $configData->method('isDemoEnabled')->willReturn(false); + $configData->method('getPasswordSalt')->willReturn('the-password-salt'); + $configData->method('isAuthBasicEnabled')->willReturn(false); + + return $configData; + } +} diff --git a/tests/Unit/Infrastructure/Adapter/In/Web/Controllers/ConfigAuth/RefusalsTest.php b/tests/Unit/Infrastructure/Adapter/In/Web/Controllers/ConfigAuth/RefusalsTest.php index 24d49ca66..1e0f7f1e4 100644 --- a/tests/Unit/Infrastructure/Adapter/In/Web/Controllers/ConfigAuth/RefusalsTest.php +++ b/tests/Unit/Infrastructure/Adapter/In/Web/Controllers/ConfigAuth/RefusalsTest.php @@ -27,6 +27,7 @@ namespace SP\Tests\Unit\Infrastructure\Adapter\In\Web\Controllers\ConfigAuth; +use SP\Application\Config\Ports\ConfigBackupService; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\Exception; @@ -93,7 +94,8 @@ public function savingIsRefusedWhenTheAclDenies(): void new SaveController( $application, - $this->simpleControllerHelper($acl, 'configAuth', 'save') + $this->simpleControllerHelper($acl, 'configAuth', 'save'), + self::createStub(ConfigBackupService::class) ); } @@ -116,7 +118,8 @@ public function savingReportsAFailureBehindItRatherThanEscaping(): void $response = (new SaveController( $application, - $this->simpleControllerHelper($acl, 'configAuth', 'save') + $this->simpleControllerHelper($acl, 'configAuth', 'save'), + self::createStub(ConfigBackupService::class) ))->saveAction(); self::assertSame(ResponseStatus::ERROR, $response->status); diff --git a/tests/Unit/Infrastructure/Adapter/In/Web/Controllers/ConfigEvents/RefusalsTest.php b/tests/Unit/Infrastructure/Adapter/In/Web/Controllers/ConfigEvents/RefusalsTest.php index 8bac5a737..666b5a6f1 100644 --- a/tests/Unit/Infrastructure/Adapter/In/Web/Controllers/ConfigEvents/RefusalsTest.php +++ b/tests/Unit/Infrastructure/Adapter/In/Web/Controllers/ConfigEvents/RefusalsTest.php @@ -27,6 +27,7 @@ namespace SP\Tests\Unit\Infrastructure\Adapter\In\Web\Controllers\ConfigEvents; +use SP\Application\Config\Ports\ConfigBackupService; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\Exception; @@ -93,7 +94,8 @@ public function savingIsRefusedWhenTheAclDenies(): void new SaveController( $application, - $this->simpleControllerHelper($acl, 'configEvents', 'save') + $this->simpleControllerHelper($acl, 'configEvents', 'save'), + self::createStub(ConfigBackupService::class) ); } @@ -116,7 +118,8 @@ public function savingReportsAFailureBehindItRatherThanEscaping(): void $response = (new SaveController( $application, - $this->simpleControllerHelper($acl, 'configEvents', 'save') + $this->simpleControllerHelper($acl, 'configEvents', 'save'), + self::createStub(ConfigBackupService::class) ))->saveAction(); self::assertSame(ResponseStatus::ERROR, $response->status); diff --git a/tests/Unit/Infrastructure/Adapter/In/Web/Controllers/ConfigGeneral/RefusalsTest.php b/tests/Unit/Infrastructure/Adapter/In/Web/Controllers/ConfigGeneral/RefusalsTest.php index 05dc5d6fd..42cac3ae8 100644 --- a/tests/Unit/Infrastructure/Adapter/In/Web/Controllers/ConfigGeneral/RefusalsTest.php +++ b/tests/Unit/Infrastructure/Adapter/In/Web/Controllers/ConfigGeneral/RefusalsTest.php @@ -139,7 +139,8 @@ public function savingIsRefusedWhenTheAclDenies(): void new SaveController( $application, $this->simpleControllerHelper($acl, 'configGeneral', 'save'), - $appLock + $appLock, + self::createStub(ConfigBackupService::class) ); } @@ -165,7 +166,8 @@ public function savingReportsAFailureBehindItRatherThanEscaping(): void $response = (new SaveController( $application, $this->simpleControllerHelper($acl, 'configGeneral', 'save'), - $appLock + $appLock, + self::createStub(ConfigBackupService::class) ))->saveAction(); self::assertSame(ResponseStatus::ERROR, $response->status); diff --git a/tests/Unit/Infrastructure/Adapter/In/Web/Controllers/ConfigLdap/RefusalsTest.php b/tests/Unit/Infrastructure/Adapter/In/Web/Controllers/ConfigLdap/RefusalsTest.php index a4f0292b3..a0f667776 100644 --- a/tests/Unit/Infrastructure/Adapter/In/Web/Controllers/ConfigLdap/RefusalsTest.php +++ b/tests/Unit/Infrastructure/Adapter/In/Web/Controllers/ConfigLdap/RefusalsTest.php @@ -27,6 +27,7 @@ namespace SP\Tests\Unit\Infrastructure\Adapter\In\Web\Controllers\ConfigLdap; +use SP\Application\Config\Ports\ConfigBackupService; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\Exception; @@ -169,7 +170,8 @@ public function savingIsRefusedWhenTheAclDenies(): void new SaveController( $application, - $this->simpleControllerHelper($acl, 'configLdap', 'save') + $this->simpleControllerHelper($acl, 'configLdap', 'save'), + self::createStub(ConfigBackupService::class) ); } @@ -195,7 +197,8 @@ public function savingReportsAFailureBehindItRatherThanEscaping(): void $response = (new SaveController( $application, - $this->simpleControllerHelper($acl, 'configLdap', 'save') + $this->simpleControllerHelper($acl, 'configLdap', 'save'), + self::createStub(ConfigBackupService::class) ))->saveAction(); self::assertSame(ResponseStatus::ERROR, $response->status); @@ -258,7 +261,9 @@ public function changingTheProfileLdapUsersGetIsRefusedWithoutThatPermission(): 'save', enablingLdap: true ) - ))->saveAction(); + , + self::createStub(ConfigBackupService::class) + ))->saveAction(); } /** @@ -278,7 +283,9 @@ public function theConnectionStillSavesWithoutThePermissionToCreateAUser(): void 'configLdap', 'save' ) - ))->saveAction(); + , + self::createStub(ConfigBackupService::class) + ))->saveAction(); // Reaching saveConfig() at all is the point: the request carries no ldap_enabled flag, so // this is the "disable it" path, which touches neither of the two guarded settings. That it diff --git a/tests/Unit/Infrastructure/Adapter/In/Web/Controllers/ConfigMail/RefusalsTest.php b/tests/Unit/Infrastructure/Adapter/In/Web/Controllers/ConfigMail/RefusalsTest.php index 1ea723bc4..aaf9bd6dc 100644 --- a/tests/Unit/Infrastructure/Adapter/In/Web/Controllers/ConfigMail/RefusalsTest.php +++ b/tests/Unit/Infrastructure/Adapter/In/Web/Controllers/ConfigMail/RefusalsTest.php @@ -27,6 +27,7 @@ namespace SP\Tests\Unit\Infrastructure\Adapter\In\Web\Controllers\ConfigMail; +use SP\Application\Config\Ports\ConfigBackupService; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\Exception; @@ -116,7 +117,8 @@ public function savingIsRefusedWhenTheAclDenies(): void new SaveController( $application, - $this->simpleControllerHelper($acl, 'configMail', 'save') + $this->simpleControllerHelper($acl, 'configMail', 'save'), + self::createStub(ConfigBackupService::class) ); } @@ -139,7 +141,8 @@ public function savingReportsAFailureBehindItRatherThanEscaping(): void $response = (new SaveController( $application, - $this->simpleControllerHelper($acl, 'configMail', 'save') + $this->simpleControllerHelper($acl, 'configMail', 'save'), + self::createStub(ConfigBackupService::class) ))->saveAction(); self::assertSame(ResponseStatus::ERROR, $response->status); diff --git a/tests/Unit/Infrastructure/Adapter/In/Web/Controllers/ConfigWiki/RefusalsTest.php b/tests/Unit/Infrastructure/Adapter/In/Web/Controllers/ConfigWiki/RefusalsTest.php index a90026e83..f5d3c724f 100644 --- a/tests/Unit/Infrastructure/Adapter/In/Web/Controllers/ConfigWiki/RefusalsTest.php +++ b/tests/Unit/Infrastructure/Adapter/In/Web/Controllers/ConfigWiki/RefusalsTest.php @@ -27,6 +27,7 @@ namespace SP\Tests\Unit\Infrastructure\Adapter\In\Web\Controllers\ConfigWiki; +use SP\Application\Config\Ports\ConfigBackupService; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\Exception; @@ -96,7 +97,8 @@ public function savingIsRefusedWhenTheAclDenies(): void new SaveController( $application, - $this->simpleControllerHelper($acl, 'configWiki', 'save') + $this->simpleControllerHelper($acl, 'configWiki', 'save'), + self::createStub(ConfigBackupService::class) ); } @@ -122,7 +124,8 @@ public function savingReportsAFailureBehindItRatherThanEscaping(): void $response = (new SaveController( $application, - $this->simpleControllerHelper($acl, 'configWiki', 'save') + $this->simpleControllerHelper($acl, 'configWiki', 'save'), + self::createStub(ConfigBackupService::class) ))->saveAction(); self::assertSame(ResponseStatus::ERROR, $response->status);