diff --git a/lib/AuditLog/AuditLogger.php b/lib/AuditLog/AuditLogger.php new file mode 100644 index 000000000..aab4597df --- /dev/null +++ b/lib/AuditLog/AuditLogger.php @@ -0,0 +1,37 @@ + $parameters named values for the placeholders of $message, in order, optionally followed by more context + */ + public function log(Operation $operation, string $message, array $parameters = []): void { + $arguments = [$message, $parameters, false]; + if (method_exists(CriticalActionPerformedEvent::class, 'getOperation')) { + $arguments[] = $operation->value; + } + $this->eventDispatcher->dispatchTyped(new CriticalActionPerformedEvent(...$arguments)); + } +} diff --git a/lib/AuditLog/Operation.php b/lib/AuditLog/Operation.php new file mode 100644 index 000000000..aebfce8e5 --- /dev/null +++ b/lib/AuditLog/Operation.php @@ -0,0 +1,58 @@ + 'An ExApp was registered and given its secret to authenticate to Nextcloud, before its deployment', + self::ExAppUpdated => 'An ExApp was switched to a new version, before the deployment of that version', + self::ExAppEnabled => 'An ExApp was enabled and started', + self::ExAppDisabled => 'An ExApp was disabled', + self::ExAppUnregistered => 'An ExApp was removed together with everything it had registered', + self::ExAppDataDeleted => 'The data volume of an ExApp was deleted on its deploy daemon', + self::ExAppAuthFailed => 'A request with AppAPI authentication was rejected for an unknown ExApp or a wrong secret', + self::DaemonRegistered => 'A deploy daemon that ExApps can be installed on was registered', + self::DaemonUpdated => 'The configuration of a deploy daemon was changed', + self::DaemonUnregistered => 'A deploy daemon was unregistered', + self::DaemonRegistryAdded => 'A deploy daemon was set to pull the images of a Docker registry from another registry', + self::DaemonRegistryRemoved => 'A Docker registry mapping was removed from a deploy daemon', + self::AdminConfigUpdated => 'An AppAPI admin setting, such as the default deploy daemon, was changed', + }; + } +} diff --git a/lib/Command/ExApp/Unregister.php b/lib/Command/ExApp/Unregister.php index fc1bd877c..000ce296b 100644 --- a/lib/Command/ExApp/Unregister.php +++ b/lib/Command/ExApp/Unregister.php @@ -112,6 +112,9 @@ protected function execute(InputInterface $input, OutputInterface $output): int return 1; } } else { + if ($rmData) { + $this->exAppService->logExAppDataDeleted($exApp); + } if (!$silent) { $output->writeln(sprintf('ExApp %s successfully removed', $appId)); } @@ -137,6 +140,9 @@ protected function execute(InputInterface $input, OutputInterface $output): int $removeVolumeResult = $this->dockerActions->removeVolume( $this->dockerActions->buildDockerUrl($daemonConfig), $volumeName ); + if (!isset($removeVolumeResult['error'])) { + $this->exAppService->logExAppDataDeleted($exApp); + } if (!$silent) { if (isset($removeVolumeResult['error'])) { $output->writeln(sprintf('Failed to remove ExApp %s volume: %s', $appId, $volumeName)); @@ -171,8 +177,13 @@ protected function execute(InputInterface $input, OutputInterface $output): int if (!$force) { return 1; } - } elseif (!$silent) { - $output->writeln(sprintf('ExApp %s K8s resources successfully removed', $appId)); + } else { + if ($rmData) { + $this->exAppService->logExAppDataDeleted($exApp); + } + if (!$silent) { + $output->writeln(sprintf('ExApp %s K8s resources successfully removed', $appId)); + } } } } diff --git a/lib/Controller/ConfigController.php b/lib/Controller/ConfigController.php index 374613391..2d0a21911 100644 --- a/lib/Controller/ConfigController.php +++ b/lib/Controller/ConfigController.php @@ -10,6 +10,8 @@ namespace OCA\AppAPI\Controller; use OCA\AppAPI\AppInfo\Application; +use OCA\AppAPI\AuditLog\AuditLogger; +use OCA\AppAPI\AuditLog\Operation; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\NoCSRFRequired; @@ -20,11 +22,14 @@ #[OpenAPI(OpenAPI::SCOPE_ADMINISTRATION)] class ConfigController extends Controller { + /** Settings whose value is written to the audit log; any other key could hold a secret, so only its name is */ + private const AUDITED_VALUE_KEYS = ['default_daemon_config', 'init_timeout', 'container_restart_policy', 'docker_api_version']; public function __construct( string $appName, IRequest $request, private readonly IAppConfig $appConfig, + private readonly AuditLogger $auditLogger, ) { parent::__construct($appName, $request); } @@ -41,7 +46,14 @@ public function __construct( #[NoCSRFRequired] public function setAdminConfig(array $values): DataResponse { foreach ($values as $key => $value) { - $this->appConfig->setValueString(Application::APP_ID, $key, $value, lazy: true); + if (!$this->appConfig->setValueString(Application::APP_ID, $key, $value, lazy: true)) { + continue; + } + if (in_array($key, self::AUDITED_VALUE_KEYS, true)) { + $this->auditLogger->log(Operation::AdminConfigUpdated, 'AppAPI setting %s was changed to %s', ['key' => $key, 'value' => $value]); + } else { + $this->auditLogger->log(Operation::AdminConfigUpdated, 'AppAPI setting %s was changed', ['key' => $key]); + } } return new DataResponse(1); } diff --git a/lib/Controller/DaemonConfigController.php b/lib/Controller/DaemonConfigController.php index 9ad23357e..969254752 100644 --- a/lib/Controller/DaemonConfigController.php +++ b/lib/Controller/DaemonConfigController.php @@ -144,7 +144,7 @@ public function updateDaemonConfig(string $name, array $daemonConfigParams): Res // Create and update DaemonConfig instance $updatedDaemonConfig = new DaemonConfig($daemonConfigParams); $updatedDaemonConfig->setId($daemonConfig->getId()); - $updatedDaemonConfig = $this->daemonConfigService->updateDaemonConfig($updatedDaemonConfig); + $updatedDaemonConfig = $this->daemonConfigService->updateDaemonConfig($updatedDaemonConfig, $daemonConfig); // Check if update was successful before proceeding if ($updatedDaemonConfig === null) { diff --git a/lib/Controller/ExAppsPageController.php b/lib/Controller/ExAppsPageController.php index 2c1133bb9..94f669bad 100644 --- a/lib/Controller/ExAppsPageController.php +++ b/lib/Controller/ExAppsPageController.php @@ -491,12 +491,18 @@ public function uninstallApp(string $appId, bool $removeContainer = true, bool $ if ($daemonConfig->getAcceptsDeployId() === $this->dockerActions->getAcceptsDeployId()) { $this->dockerActions->initGuzzleClient($daemonConfig); if (boolval($exApp->getDeployConfig()['harp'] ?? false)) { - $this->dockerActions->removeExApp($this->dockerActions->buildDockerUrl($daemonConfig), $exApp->getAppid(), removeData: $removeData); + $removeError = $this->dockerActions->removeExApp($this->dockerActions->buildDockerUrl($daemonConfig), $exApp->getAppid(), removeData: $removeData); + if ($removeData && $removeError === '') { + $this->exAppService->logExAppDataDeleted($exApp); + } } else { if ($removeContainer) { $this->dockerActions->removeContainer($this->dockerActions->buildDockerUrl($daemonConfig), $this->dockerActions->buildExAppContainerName($appId)); if ($removeData) { - $this->dockerActions->removeVolume($this->dockerActions->buildDockerUrl($daemonConfig), $this->dockerActions->buildExAppVolumeName($appId)); + $removeVolumeResult = $this->dockerActions->removeVolume($this->dockerActions->buildDockerUrl($daemonConfig), $this->dockerActions->buildExAppVolumeName($appId)); + if (isset($removeVolumeResult['success'])) { + $this->exAppService->logExAppDataDeleted($exApp); + } } } } diff --git a/lib/Service/AppAPIService.php b/lib/Service/AppAPIService.php index 1d86f299a..a8ceba5c4 100644 --- a/lib/Service/AppAPIService.php +++ b/lib/Service/AppAPIService.php @@ -10,6 +10,8 @@ namespace OCA\AppAPI\Service; use OCA\AppAPI\AppInfo\Application; +use OCA\AppAPI\AuditLog\AuditLogger; +use OCA\AppAPI\AuditLog\Operation; use OCA\AppAPI\Db\DaemonConfig; use OCA\AppAPI\Db\ExApp; use OCA\AppAPI\DeployActions\DockerActions; @@ -53,6 +55,7 @@ public function __construct( private readonly DaemonConfigService $daemonConfigService, private readonly ExAppDeployOptionsService $exAppDeployOptionsService, private readonly HarpService $harpService, + private readonly AuditLogger $auditLogger, ) { $this->client = $clientService->newClient(); $this->exAppService->setAppAPIService($this); @@ -287,17 +290,20 @@ public function validateExAppRequestToNC(IRequest $request, bool $isDav = false) $exApp = $this->exAppService->getExApp($exAppId); if ($exApp === null) { $this->logger->error(sprintf('ExApp with appId %s not found.', $request->getHeader('EX-APP-ID'))); + $userId = explode(':', base64_decode($request->getHeader('AUTHORIZATION-APP-API')), 2)[0]; // Protection for guessing installed ExApps list $this->throttler->registerAttempt(Application::APP_ID, $request->getRemoteAddress(), [ 'appid' => $request->getHeader('EX-APP-ID'), - 'userid' => explode(':', base64_decode($request->getHeader('AUTHORIZATION-APP-API')), 2)[0], + 'userid' => $userId, ]); + $this->logAuthenticationFailure($exAppId, $userId, 'unknown_ex_app'); return false; } $authorization = base64_decode($request->getHeader('AUTHORIZATION-APP-API')); if ($authorization === false) { $this->logger->error('Failed to parse AUTHORIZATION-APP-API'); + $this->logAuthenticationFailure($exAppId, '', 'malformed_authorization'); return false; } $userId = explode(':', $authorization, 2)[0]; @@ -325,12 +331,24 @@ public function validateExAppRequestToNC(IRequest $request, bool $isDav = false) 'appid' => $request->getHeader('EX-APP-ID'), 'userid' => $userId, ]); + $this->logAuthenticationFailure($exApp->getAppid(), $userId, 'invalid_secret'); } $this->logger->error(sprintf('ExApp %s request to NC validation failed.', $exApp->getAppid())); return false; } + /** + * @param string $userId the user the request claims to act for, as sent by the client + */ + private function logAuthenticationFailure(string $exAppId, string $userId, string $reason): void { + $this->auditLogger->log(Operation::ExAppAuthFailed, 'Failed AppAPI authentication of ExApp %s for user %s', [ + 'exAppId' => $exAppId, + 'userId' => $userId, + 'reason' => $reason, + ]); + } + /** * Final step of AppAPI authentication request validation for Nextcloud: * - sets active user (null if not a user context) @@ -704,6 +722,9 @@ public function enableExApp(ExApp $exApp): bool { } $this->harpService->harpExAppUpdate($daemonConfig, $exApp, true); + $this->auditLogger->log(Operation::ExAppEnabled, 'ExApp %s was enabled', [ + 'appId' => $exApp->getAppid(), + ]); return true; } @@ -754,7 +775,12 @@ public function disableExApp(ExApp $exApp): bool { } } } - $this->exAppService->disableExAppInternal($exApp); + // The ExApp is disabled even when it could not be told about it + if ($this->exAppService->disableExAppInternal($exApp)) { + $this->auditLogger->log(Operation::ExAppDisabled, 'ExApp %s was disabled', [ + 'appId' => $exApp->getAppid(), + ]); + } if ($result) { $this->harpService->harpExAppUpdate($daemonConfig, $exApp, false); @@ -803,10 +829,13 @@ public function removeExAppsByDaemonConfigName(DaemonConfig $daemonConfig): void if ($daemonConfig->getAcceptsDeployId() === $this->dockerActions->getAcceptsDeployId()) { $this->dockerActions->initGuzzleClient($daemonConfig); if (boolval($exApp->getDeployConfig()['harp'] ?? false)) { - $this->dockerActions->removeExApp($this->dockerActions->buildDockerUrl($daemonConfig), $exApp->getAppid(), true); + $dataDeleted = $this->dockerActions->removeExApp($this->dockerActions->buildDockerUrl($daemonConfig), $exApp->getAppid(), true) === ''; } else { $this->dockerActions->removeContainer($this->dockerActions->buildDockerUrl($daemonConfig), $this->dockerActions->buildExAppContainerName($exApp->getAppid())); - $this->dockerActions->removeVolume($this->dockerActions->buildDockerUrl($daemonConfig), $this->dockerActions->buildExAppVolumeName($exApp->getAppid())); + $dataDeleted = isset($this->dockerActions->removeVolume($this->dockerActions->buildDockerUrl($daemonConfig), $this->dockerActions->buildExAppVolumeName($exApp->getAppid()))['success']); + } + if ($dataDeleted) { + $this->exAppService->logExAppDataDeleted($exApp); } } $this->exAppService->unregisterExApp($exApp->getAppid()); diff --git a/lib/Service/DaemonConfigService.php b/lib/Service/DaemonConfigService.php index aa920d648..592e070a6 100644 --- a/lib/Service/DaemonConfigService.php +++ b/lib/Service/DaemonConfigService.php @@ -9,6 +9,8 @@ namespace OCA\AppAPI\Service; +use OCA\AppAPI\AuditLog\AuditLogger; +use OCA\AppAPI\AuditLog\Operation; use OCA\AppAPI\Db\DaemonConfig; use OCA\AppAPI\Db\DaemonConfigMapper; use OCA\AppAPI\DeployActions\DockerActions; @@ -36,6 +38,7 @@ public function __construct( private DaemonConfigMapper $mapper, private ExAppService $exAppService, private ICrypto $crypto, + private AuditLogger $auditLogger, ) { } @@ -134,7 +137,7 @@ public function registerDaemonConfig(array $params): ?DaemonConfig { if (isset($params['deploy_config']['haproxy_password']) && $params['deploy_config']['haproxy_password'] !== '') { $params['deploy_config']['haproxy_password'] = $this->crypto->encrypt($params['deploy_config']['haproxy_password']); } - return $this->mapper->insert(new DaemonConfig([ + $daemonConfig = $this->mapper->insert(new DaemonConfig([ 'name' => $params['name'], 'display_name' => $params['display_name'], 'accepts_deploy_id' => $params['accepts_deploy_id'], @@ -146,15 +149,25 @@ public function registerDaemonConfig(array $params): ?DaemonConfig { $this->logger->error('Failed to register daemon config. Error: ' . $e->getMessage(), ['exception' => $e]); return null; } + $this->auditLogger->log(Operation::DaemonRegistered, 'Deploy daemon %s was registered', [ + 'daemonName' => $daemonConfig->getName(), + 'acceptsDeployId' => $daemonConfig->getAcceptsDeployId(), + 'host' => $daemonConfig->getHost(), + ]); + return $daemonConfig; } public function unregisterDaemonConfig(DaemonConfig $daemonConfig): ?DaemonConfig { try { - return $this->mapper->delete($daemonConfig); + $daemonConfig = $this->mapper->delete($daemonConfig); } catch (Exception $e) { $this->logger->error('Failed to unregister daemon config. Error: ' . $e->getMessage(), ['exception' => $e]); return null; } + $this->auditLogger->log(Operation::DaemonUnregistered, 'Deploy daemon %s was unregistered', [ + 'daemonName' => $daemonConfig->getName(), + ]); + return $daemonConfig; } /** @@ -202,7 +215,10 @@ public function getDaemonConfigByName(string $name): ?DaemonConfig { } } - public function updateDaemonConfig(DaemonConfig $daemonConfig): ?DaemonConfig { + /** + * @param DaemonConfig $previousDaemonConfig the stored configuration before the update + */ + public function updateDaemonConfig(DaemonConfig $daemonConfig, DaemonConfig $previousDaemonConfig): ?DaemonConfig { $name = $daemonConfig->getName() ?? ''; if ($name === '' || $this->containsControlCharacters($name)) { $this->logger->error('Failed to update daemon configuration: `name` contains invalid characters or is empty.'); @@ -210,11 +226,20 @@ public function updateDaemonConfig(DaemonConfig $daemonConfig): ?DaemonConfig { } try { - return $this->mapper->update($daemonConfig); + $daemonConfig = $this->mapper->update($daemonConfig); } catch (Exception $e) { $this->logger->error('Failed to update DaemonConfig. Error: ' . $e->getMessage(), ['exception' => $e]); return null; } + // Only whether the secret changed is logged, never the secret itself + $secretChanged = ($daemonConfig->getDeployConfig()['haproxy_password'] ?? '') + !== ($previousDaemonConfig->getDeployConfig()['haproxy_password'] ?? ''); + $this->auditLogger->log(Operation::DaemonUpdated, 'Deploy daemon %s was updated', [ + 'daemonName' => $daemonConfig->getName(), + 'host' => $daemonConfig->getHost(), + 'secretChanged' => $secretChanged, + ]); + return $daemonConfig; } public function addDockerRegistry(DaemonConfig $daemonConfig, array $registryMap): DaemonConfig|array|null { @@ -246,11 +271,17 @@ public function addDockerRegistry(DaemonConfig $daemonConfig, array $registryMap $deployConfig['registries'] = [...array_values($deployConfig['registries']), ['from' => $from, 'to' => $to]]; $daemonConfig->setDeployConfig($deployConfig); - return $this->mapper->update($daemonConfig); + $daemonConfig = $this->mapper->update($daemonConfig); } catch (Exception $e) { $this->logger->error('Failed to add registry to DaemonConfig. Error: ' . $e->getMessage(), ['exception' => $e]); return null; } + $this->auditLogger->log(Operation::DaemonRegistryAdded, 'Docker registry mapping was added to deploy daemon %s: %s to %s', [ + 'daemonName' => $daemonConfig->getName(), + 'from' => $from, + 'to' => $to, + ]); + return $daemonConfig; } public function removeDockerRegistry(DaemonConfig $daemonConfig, array $registryMap): DaemonConfig|array|null { @@ -265,11 +296,17 @@ public function removeDockerRegistry(DaemonConfig $daemonConfig, array $registry })); $daemonConfig->setDeployConfig($deployConfig); - return $this->mapper->update($daemonConfig); + $daemonConfig = $this->mapper->update($daemonConfig); } catch (Exception $e) { $this->logger->error('Failed to remove registry from DaemonConfig. Error: ' . $e->getMessage(), ['exception' => $e]); return null; } + $this->auditLogger->log(Operation::DaemonRegistryRemoved, 'Docker registry mapping was removed from deploy daemon %s: %s to %s', [ + 'daemonName' => $daemonConfig->getName(), + 'from' => $registryMap['from'] ?? null, + 'to' => $registryMap['to'] ?? null, + ]); + return $daemonConfig; } /** diff --git a/lib/Service/ExAppService.php b/lib/Service/ExAppService.php index d39fab8c4..83d39b208 100644 --- a/lib/Service/ExAppService.php +++ b/lib/Service/ExAppService.php @@ -11,6 +11,8 @@ use InvalidArgumentException; use OCA\AppAPI\AppInfo\Application; +use OCA\AppAPI\AuditLog\AuditLogger; +use OCA\AppAPI\AuditLog\Operation; use OCA\AppAPI\Db\ExApp; use OCA\AppAPI\Db\ExAppMapper; use OCA\AppAPI\Fetcher\ExAppArchiveFetcher; @@ -58,6 +60,7 @@ public function __construct( private readonly ExAppDeployOptionsService $deployOptionsService, private readonly ExAppSetupCheckService $setupCheckService, private readonly IConfig $config, + private readonly AuditLogger $auditLogger, ) { if ($cacheFactory->isAvailable()) { $distributedCacheClass = ltrim($config->getSystemValueString('memcache.distributed', ''), '\\'); @@ -100,6 +103,12 @@ public function registerExApp(array $appInfo): ?ExApp { if (isset($appInfo['external-app']['routes'])) { $exApp->setRoutes($this->registerExAppRoutes($exApp, $appInfo['external-app']['routes'])->getRoutes() ?? []); } + $this->auditLogger->log(Operation::ExAppRegistered, 'ExApp %s (version %s) was registered on deploy daemon %s', [ + 'appId' => $exApp->getAppid(), + 'version' => $exApp->getVersion(), + 'daemonName' => $exApp->getDaemonConfigName(), + 'port' => $exApp->getPort(), + ]); return $exApp; } catch (Exception|MultipleObjectsReturnedException|DoesNotExistException $e) { $this->logger->error(sprintf('Error while registering ExApp %s: %s', $appInfo['id'], $e->getMessage())); @@ -128,6 +137,10 @@ public function unregisterExApp(string $appId): bool { $r = $this->exAppMapper->deleteExApp($appId); if ($r !== 1) { $this->logger->error(sprintf('Error while unregistering %s ExApp from the database.', $appId)); + } else { + $this->auditLogger->log(Operation::ExAppUnregistered, 'ExApp %s was unregistered', [ + 'appId' => $appId, + ]); } $rmRoutes = $this->removeExAppRoutes($exApp); if ($rmRoutes === null) { @@ -158,9 +171,19 @@ public function enableExAppInternal(ExApp $exApp): bool { return $this->updateExApp($exApp, ['enabled', 'status']); } - public function disableExAppInternal(ExApp $exApp): void { + public function disableExAppInternal(ExApp $exApp): bool { $exApp->setEnabled(0); - $this->updateExApp($exApp, ['enabled']); + return $this->updateExApp($exApp, ['enabled']); + } + + /** + * Logs the deletion of the data volume of an ExApp on its deploy daemon + */ + public function logExAppDataDeleted(ExApp $exApp): void { + $this->auditLogger->log(Operation::ExAppDataDeleted, 'Data of ExApp %s was deleted from deploy daemon %s', [ + 'appId' => $exApp->getAppid(), + 'daemonName' => $exApp->getDaemonConfigName(), + ]); } public function getExAppsByDaemonName(string $daemonName): array { @@ -203,11 +226,17 @@ public function getNCUsersList(): ?array { } public function updateExAppInfo(ExApp $exApp, array $appInfo): bool { + $oldVersion = $exApp->getVersion(); $exApp->setVersion($appInfo['version']); $exApp->setName($appInfo['name']); if (!$this->updateExApp($exApp, ['version', 'name'])) { return false; } + $this->auditLogger->log(Operation::ExAppUpdated, 'ExApp %s was updated from version %s to %s', [ + 'appId' => $exApp->getAppid(), + 'oldVersion' => $oldVersion, + 'newVersion' => $exApp->getVersion(), + ]); return true; } diff --git a/tests/php/AuditLog/AuditEvents.php b/tests/php/AuditLog/AuditEvents.php new file mode 100644 index 000000000..ec87832cf --- /dev/null +++ b/tests/php/AuditLog/AuditEvents.php @@ -0,0 +1,21 @@ +getOperation() === $operation; + } +} diff --git a/tests/php/AuditLog/AuditLoggerTest.php b/tests/php/AuditLog/AuditLoggerTest.php new file mode 100644 index 000000000..096880e1a --- /dev/null +++ b/tests/php/AuditLog/AuditLoggerTest.php @@ -0,0 +1,33 @@ +createMock(IEventDispatcher::class); + $eventDispatcher->expects(self::once()) + ->method('dispatchTyped') + ->with(self::callback(static function (CriticalActionPerformedEvent $event): bool { + self::assertSame('Deploy daemon %s was unregistered', $event->getLogMessage()); + self::assertSame(['daemonName' => 'harp_proxy'], $event->getParameters()); + self::assertFalse($event->getObfuscateParameters()); + self::assertTrue(AuditEvents::hasOperation($event, 'app_api.daemon.unregistered')); + return true; + })); + + (new AuditLogger($eventDispatcher))->log(Operation::DaemonUnregistered, 'Deploy daemon %s was unregistered', ['daemonName' => 'harp_proxy']); + } +} diff --git a/tests/php/AuditLog/OperationTest.php b/tests/php/AuditLog/OperationTest.php new file mode 100644 index 000000000..e24b46952 --- /dev/null +++ b/tests/php/AuditLog/OperationTest.php @@ -0,0 +1,26 @@ + [$operation], Operation::cases()); + } + + #[DataProvider('dataOperations')] + public function testOperationFormat(Operation $operation): void { + self::assertMatchesRegularExpression('/^app_api\.[a-z][a-z0-9_]*\.[a-z][a-z0-9_]*$/', $operation->value); + self::assertNotSame('', $operation->description()); + } +} diff --git a/tests/php/Command/ExApp/UnregisterTest.php b/tests/php/Command/ExApp/UnregisterTest.php new file mode 100644 index 000000000..69a1da6a4 --- /dev/null +++ b/tests/php/Command/ExApp/UnregisterTest.php @@ -0,0 +1,126 @@ +daemonConfigService = $this->createMock(DaemonConfigService::class); + $this->dockerActions = $this->createMock(DockerActions::class); + $this->dockerActions->method('getAcceptsDeployId')->willReturn('docker-install'); + $this->kubernetesActions = $this->createMock(KubernetesActions::class); + $this->kubernetesActions->method('getAcceptsDeployId')->willReturn('kubernetes-install'); + $this->exAppService = $this->createMock(ExAppService::class); + $this->exAppService->method('unregisterExApp')->willReturn(true); + + $this->commandTester = new CommandTester(new Unregister( + $this->createMock(AppAPIService::class), + $this->daemonConfigService, + $this->dockerActions, + $this->kubernetesActions, + $this->exAppService, + $this->createMock(ExAppDeployOptionsService::class), + )); + } + + private function registerExApp(string $acceptsDeployId, bool $isHarp): ExApp { + $exApp = new ExApp([ + 'appid' => 'test_app', + 'daemon_config_name' => 'test_daemon', + 'enabled' => 0, + ]); + $exApp->setDeployConfig($isHarp ? ['harp' => ['frp_address' => 'appapi-harp:8782']] : []); + $this->exAppService->method('getExApp')->with('test_app')->willReturn($exApp); + $this->daemonConfigService->method('getDaemonConfigByName') + ->willReturn(new DaemonConfig(['name' => 'test_daemon', 'accepts_deploy_id' => $acceptsDeployId])); + return $exApp; + } + + public static function harpProvider(): array { + return [ + 'removed with its data' => [true, '', true], + 'removed without its data' => [false, '', false], + 'removal failed' => [true, 'Failed to remove ExApp', false], + ]; + } + + #[DataProvider('harpProvider')] + public function testHarpExAppDataDeletionIsLogged(bool $rmData, string $removeError, bool $logged): void { + $exApp = $this->registerExApp('docker-install', true); + $this->dockerActions->method('removeExApp')->willReturn($removeError); + $this->exAppService->expects($logged ? self::once() : self::never())->method('logExAppDataDeleted')->with($exApp); + + $this->commandTester->execute(['appid' => 'test_app', '--rm-data' => $rmData, '--force' => true, '--silent' => true]); + } + + public static function dockerSocketProxyProvider(): array { + return [ + 'volume removed' => [['success' => true], true], + 'volume not removed' => [['error' => 'Volume is in use.'], false], + ]; + } + + #[DataProvider('dockerSocketProxyProvider')] + public function testDockerSocketProxyExAppDataDeletionIsLogged(array $removeVolumeResult, bool $logged): void { + $exApp = $this->registerExApp('docker-install', false); + $this->dockerActions->method('removeContainer')->willReturn(''); + $this->dockerActions->method('removeVolume')->willReturn($removeVolumeResult); + $this->exAppService->expects($logged ? self::once() : self::never())->method('logExAppDataDeleted')->with($exApp); + + $this->commandTester->execute(['appid' => 'test_app', '--rm-data' => true, '--silent' => true]); + } + + public function testDockerSocketProxyExAppDataIsKeptByDefault(): void { + $this->registerExApp('docker-install', false); + $this->dockerActions->method('removeContainer')->willReturn(''); + $this->dockerActions->expects(self::never())->method('removeVolume'); + $this->exAppService->expects(self::never())->method('logExAppDataDeleted'); + + $this->commandTester->execute(['appid' => 'test_app', '--silent' => true]); + } + + public static function kubernetesProvider(): array { + return [ + 'removed with its data' => [true, '', true], + 'removed without its data' => [false, '', false], + 'removal failed' => [true, 'K8s deployment not found', false], + ]; + } + + #[DataProvider('kubernetesProvider')] + public function testKubernetesExAppDataDeletionIsLogged(bool $rmData, string $removeError, bool $logged): void { + $exApp = $this->registerExApp('kubernetes-install', true); + $this->kubernetesActions->method('removeExApp')->willReturn($removeError); + $this->exAppService->expects($logged ? self::once() : self::never())->method('logExAppDataDeleted')->with($exApp); + + $this->commandTester->execute(['appid' => 'test_app', '--rm-data' => $rmData, '--force' => true, '--silent' => true]); + } +} diff --git a/tests/php/Controller/ConfigControllerTest.php b/tests/php/Controller/ConfigControllerTest.php new file mode 100644 index 000000000..710368c54 --- /dev/null +++ b/tests/php/Controller/ConfigControllerTest.php @@ -0,0 +1,68 @@ +appConfig = $this->createMock(IAppConfig::class); + $this->auditLogger = $this->createMock(AuditLogger::class); + $this->controller = new ConfigController('app_api', $this->createMock(IRequest::class), $this->appConfig, $this->auditLogger); + } + + public function testSetAdminConfigLogsEachChangedSetting(): void { + $this->appConfig->expects(self::exactly(3))->method('setValueString') + ->willReturnCallback(static fn (string $app, string $key, string $value): bool => $key !== 'init_timeout'); + $logged = []; + $this->auditLogger->expects(self::exactly(2))->method('log') + ->willReturnCallback(static function (Operation $operation, string $message, array $parameters) use (&$logged): void { + $logged[] = [$operation, $message, $parameters]; + }); + + $this->controller->setAdminConfig([ + 'default_daemon_config' => 'harp_proxy', + 'init_timeout' => '40', + 'container_restart_policy' => 'always', + ]); + + self::assertSame([ + [Operation::AdminConfigUpdated, 'AppAPI setting %s was changed to %s', ['key' => 'default_daemon_config', 'value' => 'harp_proxy']], + [Operation::AdminConfigUpdated, 'AppAPI setting %s was changed to %s', ['key' => 'container_restart_policy', 'value' => 'always']], + ], $logged); + } + + public function testSetAdminConfigDoesNotLogTheValueOfAnUnknownSetting(): void { + $this->appConfig->method('setValueString')->willReturn(true); + $this->auditLogger->expects(self::once())->method('log') + ->with(Operation::AdminConfigUpdated, 'AppAPI setting %s was changed', ['key' => 'some_token']); + + $this->controller->setAdminConfig(['some_token' => 'a secret value']); + } + + public function testSetAdminConfigDoesNotLogAnUnchangedSetting(): void { + $this->appConfig->method('setValueString')->willReturn(false); + $this->auditLogger->expects(self::never())->method('log'); + + $this->controller->setAdminConfig(['init_timeout' => '40']); + } +} diff --git a/tests/php/Controller/DaemonConfigControllerTest.php b/tests/php/Controller/DaemonConfigControllerTest.php index efe1a8cab..aaf1417d5 100644 --- a/tests/php/Controller/DaemonConfigControllerTest.php +++ b/tests/php/Controller/DaemonConfigControllerTest.php @@ -119,12 +119,13 @@ public function testUpdateRejectsAnInvalidNewSecret(): void { public function testUpdateEncryptsAValidNewSecret(): void { $params = self::harpDaemonParams('a_long_enough_key'); + $storedDaemon = self::storedDaemon('encrypted:stored_secret_1', true); $this->daemonConfigService->method('getDaemonConfigByName') - ->willReturn(self::storedDaemon('encrypted:stored_secret_1', true)); + ->willReturn($storedDaemon); $this->daemonConfigService->expects(self::once())->method('validateNewSecret')->willReturn(null); $this->daemonConfigService->expects(self::once())->method('updateDaemonConfig') ->with(self::callback(static fn (DaemonConfig $daemonConfig): bool - => $daemonConfig->getDeployConfig()['haproxy_password'] === 'encrypted:a_long_enough_key')) + => $daemonConfig->getDeployConfig()['haproxy_password'] === 'encrypted:a_long_enough_key'), self::identicalTo($storedDaemon)) ->willReturnArgument(0); $response = $this->controller->updateDaemonConfig('harp_proxy_host', $params); diff --git a/tests/php/Controller/ExAppsPageControllerTest.php b/tests/php/Controller/ExAppsPageControllerTest.php index 0a8c366b0..916337108 100644 --- a/tests/php/Controller/ExAppsPageControllerTest.php +++ b/tests/php/Controller/ExAppsPageControllerTest.php @@ -11,6 +11,8 @@ use OC\App\AppStore\Fetcher\CategoryFetcher; use OCA\AppAPI\Controller\ExAppsPageController; +use OCA\AppAPI\Db\DaemonConfig; +use OCA\AppAPI\Db\ExApp; use OCA\AppAPI\DeployActions\DockerActions; use OCA\AppAPI\Fetcher\ExAppFetcher; use OCA\AppAPI\Service\AppAPIService; @@ -23,6 +25,7 @@ use OCP\IL10N; use OCP\IRequest; use OCP\L10N\IFactory; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; @@ -36,6 +39,7 @@ class ExAppsPageControllerTest extends TestCase { private DaemonConfigService&MockObject $daemonConfigService; private IConfig&MockObject $config; private IAppManager&MockObject $appManager; + private DockerActions&MockObject $dockerActions; protected function setUp(): void { parent::setUp(); @@ -44,7 +48,7 @@ protected function setUp(): void { $this->config = $this->createMock(IConfig::class); $service = $this->createMock(AppAPIService::class); $this->daemonConfigService = $this->createMock(DaemonConfigService::class); - $dockerActions = $this->createMock(DockerActions::class); + $this->dockerActions = $this->createMock(DockerActions::class); $categoryFetcher = $this->createMock(CategoryFetcher::class); $this->l10nFactory = $this->createMock(IFactory::class); $this->exAppFetcher = $this->createMock(ExAppFetcher::class); @@ -59,7 +63,7 @@ protected function setUp(): void { $this->config, $service, $this->daemonConfigService, - $dockerActions, + $this->dockerActions, $categoryFetcher, $this->l10nFactory, $this->exAppFetcher, @@ -228,4 +232,36 @@ public function testForceRecoversFromNonArrayOverwriteValue(): void { self::assertInstanceOf(JSONResponse::class, $this->controller->force('my_exapp')); } + + public static function uninstallDataProvider(): array { + return [ + 'HaRP, data removed' => [true, true, '', null, true], + 'HaRP, data kept' => [true, false, '', null, false], + 'HaRP, removal failed' => [true, true, 'Failed to remove ExApp', null, false], + 'Docker Socket Proxy, volume removed' => [false, true, '', ['success' => true], true], + 'Docker Socket Proxy, volume not removed' => [false, true, '', ['error' => 'Volume is in use.'], false], + 'Docker Socket Proxy, data kept' => [false, false, '', null, false], + ]; + } + + #[DataProvider('uninstallDataProvider')] + public function testUninstallAppLogsTheDeletedData(bool $isHarp, bool $removeData, string $removeExAppError, ?array $removeVolumeResult, bool $logged): void { + $exApp = new ExApp([ + 'appid' => 'fake_app', + 'daemon_config_name' => 'docker_daemon', + 'enabled' => 0, + ]); + $exApp->setDeployConfig($isHarp ? ['harp' => ['frp_address' => 'appapi-harp:8782']] : []); + $this->exAppService->method('getExApp')->with('fake_app')->willReturn($exApp); + $this->daemonConfigService->method('getDaemonConfigByName') + ->willReturn(new DaemonConfig(['name' => 'docker_daemon', 'accepts_deploy_id' => 'docker-install'])); + $this->dockerActions->method('getAcceptsDeployId')->willReturn('docker-install'); + $this->dockerActions->method('removeExApp')->willReturn($removeExAppError); + $this->dockerActions->expects($removeData && !$isHarp ? self::once() : self::never()) + ->method('removeVolume')->willReturn($removeVolumeResult ?? []); + $this->exAppService->expects($logged ? self::once() : self::never())->method('logExAppDataDeleted')->with($exApp); + $this->exAppService->expects(self::once())->method('unregisterExApp')->with('fake_app'); + + $this->controller->uninstallApp('fake_app', true, $removeData); + } } diff --git a/tests/php/Service/AppAPIServiceTest.php b/tests/php/Service/AppAPIServiceTest.php index bc2ef7a84..6bec8557d 100644 --- a/tests/php/Service/AppAPIServiceTest.php +++ b/tests/php/Service/AppAPIServiceTest.php @@ -9,6 +9,9 @@ namespace OCA\AppAPI\Tests\php\Service; +use OCA\AppAPI\AuditLog\AuditLogger; +use OCA\AppAPI\AuditLog\Operation; +use OCA\AppAPI\Db\DaemonConfig; use OCA\AppAPI\Db\ExApp; use OCA\AppAPI\DeployActions\DockerActions; use OCA\AppAPI\DeployActions\KubernetesActions; @@ -21,12 +24,14 @@ use OCA\AppAPI\Service\HarpService; use OCP\Http\Client\IClient; use OCP\Http\Client\IClientService; +use OCP\Http\Client\IResponse; use OCP\IConfig; use OCP\ISession; use OCP\IUserManager; use OCP\IUserSession; use OCP\L10N\IFactory; use OCP\Security\Bruteforce\IThrottler; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; @@ -48,6 +53,8 @@ class AppAPIServiceTest extends TestCase { private IUserSession&MockObject $userSession; private ISession&MockObject $session; private IUserManager&MockObject $userManager; + private DaemonConfigService&MockObject $daemonConfigService; + private AuditLogger&MockObject $auditLogger; protected function setUp(): void { parent::setUp(); @@ -68,7 +75,8 @@ protected function setUp(): void { $this->kubernetesActions = $this->createMock(KubernetesActions::class); $this->manualActions = $this->createMock(ManualActions::class); $this->commonService = $this->createMock(AppAPICommonService::class); - $daemonConfigService = $this->createMock(DaemonConfigService::class); + $this->daemonConfigService = $this->createMock(DaemonConfigService::class); + $this->auditLogger = $this->createMock(AuditLogger::class); $exAppDeployOptionsService = $this->createMock(ExAppDeployOptionsService::class); $harpService = $this->createMock(HarpService::class); @@ -87,9 +95,10 @@ protected function setUp(): void { $this->kubernetesActions, $this->manualActions, $this->commonService, - $daemonConfigService, + $this->daemonConfigService, $exAppDeployOptionsService, $harpService, + $this->auditLogger, ); } @@ -269,6 +278,7 @@ public function testValidateDisabledExAppRejectedForNonExemptEndpoint(): void { $this->exAppService->method('getExApp')->with('test_app')->willReturn($exApp); $this->throttler->method('sleepDelayOrThrowOnMax')->willReturn(0); + $this->auditLogger->expects(self::never())->method('log'); self::assertFalse($this->service->validateExAppRequestToNC($request)); } @@ -283,6 +293,7 @@ public function testValidateEnabledExAppAllowedEverywhere(): void { $this->exAppService->method('getExApp')->with('test_app')->willReturn($exApp); $this->throttler->method('sleepDelayOrThrowOnMax')->willReturn(0); + $this->auditLogger->expects(self::never())->method('log'); self::assertTrue($this->service->validateExAppRequestToNC($request)); } @@ -297,7 +308,168 @@ public function testValidateInvalidSecretRejected(): void { $this->exAppService->method('getExApp')->with('test_app')->willReturn($exApp); $this->throttler->method('sleepDelayOrThrowOnMax')->willReturn(0); + $this->auditLogger->expects(self::once())->method('log')->with( + Operation::ExAppAuthFailed, + 'Failed AppAPI authentication of ExApp %s for user %s', + ['exAppId' => 'test_app', 'userId' => '', 'reason' => 'invalid_secret'], + ); self::assertFalse($this->service->validateExAppRequestToNC($request)); } + + private function createResponse(string $body, int $statusCode = 200): IResponse&MockObject { + $response = $this->createMock(IResponse::class); + $response->method('getStatusCode')->willReturn($statusCode); + $response->method('getBody')->willReturn($body); + return $response; + } + + private function setupReachableManualExApp(ExApp $exApp, string $enabledResponseBody): void { + $this->setupExAppUrlMocks(); + $this->daemonConfigService->method('getDaemonConfigByName')->willReturn(new DaemonConfig(['name' => 'test_daemon'])); + $this->exAppService->method('getExApp')->willReturn($exApp); + $this->commonService->method('buildAppAPIAuthHeaders')->willReturn([]); + $this->client->method('get')->willReturn($this->createResponse('{"status":"ok"}')); + $this->client->method('put')->willReturn($this->createResponse($enabledResponseBody)); + } + + public function testEnableExAppLogsTheExApp(): void { + $exApp = $this->createExApp(enabled: false); + $this->setupReachableManualExApp($exApp, '{}'); + $this->exAppService->method('enableExAppInternal')->willReturn(true); + $this->auditLogger->expects(self::once())->method('log') + ->with(Operation::ExAppEnabled, 'ExApp %s was enabled', ['appId' => 'test_app']); + + self::assertTrue($this->service->enableExApp($exApp)); + } + + public function testEnableExAppDoesNotLogWhenTheExAppRefuses(): void { + $exApp = $this->createExApp(enabled: false); + $this->setupReachableManualExApp($exApp, '{"error":"missing configuration"}'); + $this->exAppService->method('enableExAppInternal')->willReturn(true); + $this->exAppService->expects(self::once())->method('disableExAppInternal')->with($exApp); + $this->auditLogger->expects(self::never())->method('log'); + + self::assertFalse($this->service->enableExApp($exApp)); + } + + public function testEnableExAppDoesNotLogWhenItCannotBeStored(): void { + $exApp = $this->createExApp(enabled: false); + $this->exAppService->method('enableExAppInternal')->willReturn(false); + $this->auditLogger->expects(self::never())->method('log'); + + self::assertFalse($this->service->enableExApp($exApp)); + } + + public function testDisableExAppLogsTheExApp(): void { + $exApp = $this->createExApp(); + $this->setupReachableManualExApp($exApp, '{}'); + $this->exAppService->method('disableExAppInternal')->willReturn(true); + $this->auditLogger->expects(self::once())->method('log') + ->with(Operation::ExAppDisabled, 'ExApp %s was disabled', ['appId' => 'test_app']); + + self::assertTrue($this->service->disableExApp($exApp)); + } + + public function testDisableExAppLogsAnExAppThatCouldNotBeToldAboutIt(): void { + $exApp = $this->createExApp(); + $this->setupReachableManualExApp($exApp, '{"error":"busy"}'); + $this->exAppService->expects(self::once())->method('disableExAppInternal')->with($exApp)->willReturn(true); + $this->auditLogger->expects(self::once())->method('log') + ->with(Operation::ExAppDisabled, 'ExApp %s was disabled', ['appId' => 'test_app']); + + self::assertFalse($this->service->disableExApp($exApp)); + } + + public function testDisableExAppDoesNotLogWhenItCannotBeStored(): void { + $exApp = $this->createExApp(); + $this->setupReachableManualExApp($exApp, '{}'); + $this->exAppService->method('disableExAppInternal')->willReturn(false); + $this->auditLogger->expects(self::never())->method('log'); + + $this->service->disableExApp($exApp); + } + + public function testDisableExAppDoesNotLogWithoutItsDaemon(): void { + $exApp = $this->createExApp(); + $this->setupExAppUrlMocks(); + $this->commonService->method('buildAppAPIAuthHeaders')->willReturn([]); + $this->client->method('put')->willReturn($this->createResponse('{}')); + $this->daemonConfigService->method('getDaemonConfigByName')->willReturn(null); + $this->exAppService->expects(self::never())->method('disableExAppInternal'); + $this->auditLogger->expects(self::never())->method('log'); + + self::assertFalse($this->service->disableExApp($exApp)); + } + + public static function removeExAppsByDaemonProvider(): array { + return [ + 'HaRP, removed' => [true, '', null, true], + 'HaRP, failed' => [true, 'Failed to remove ExApp', null, false], + 'Docker Socket Proxy, volume removed' => [false, '', ['success' => true], true], + 'Docker Socket Proxy, volume not removed' => [false, '', ['error' => 'Volume is in use.'], false], + ]; + } + + #[DataProvider('removeExAppsByDaemonProvider')] + public function testRemoveExAppsByDaemonConfigNameLogsTheDeletedData(bool $isHarp, string $removeExAppError, ?array $removeVolumeResult, bool $logged): void { + $exApp = $this->createExApp(); + $exApp->setAcceptsDeployId('docker-install'); + $exApp->setDeployConfig($isHarp ? ['harp' => ['frp_address' => 'appapi-harp:8782']] : []); + $daemonConfig = new DaemonConfig(['name' => 'test_daemon', 'accepts_deploy_id' => 'docker-install']); + $this->setupReachableManualExApp($exApp, '{}'); + $this->exAppService->method('getExAppsByDaemonName')->with('test_daemon')->willReturn([$exApp]); + $this->exAppService->method('disableExAppInternal')->willReturn(true); + $this->dockerActions->method('removeExApp')->willReturn($removeExAppError); + $this->dockerActions->method('removeVolume')->willReturn($removeVolumeResult ?? []); + $this->exAppService->expects($logged ? self::once() : self::never())->method('logExAppDataDeleted')->with($exApp); + $this->exAppService->expects(self::once())->method('unregisterExApp')->with('test_app'); + + $this->service->removeExAppsByDaemonConfigName($daemonConfig); + } + + private function createRequestForUser(string $appId, string $userId, string $secret): MockObject&\OCP\IRequest { + $request = $this->createMock(\OCP\IRequest::class); + $request->method('getRemoteAddress')->willReturn('127.0.0.1'); + $request->method('getHeader')->willReturnCallback(static fn (string $name): string => match ($name) { + 'EX-APP-ID' => $appId, + 'EX-APP-VERSION' => '1.0.0', + 'AUTHORIZATION-APP-API' => base64_encode($userId . ':' . $secret), + default => '', + }); + $request->method('getPathInfo')->willReturn('/ocs/v1.php/apps/app_api/api/v1/log'); + return $request; + } + + public function testValidateLogsAWrongSecretWithTheClaimedUser(): void { + $this->exAppService->method('getExApp')->with('test_app')->willReturn($this->createExApp()); + $this->throttler->method('sleepDelayOrThrowOnMax')->willReturn(0); + $this->auditLogger->expects(self::once())->method('log')->with( + Operation::ExAppAuthFailed, + 'Failed AppAPI authentication of ExApp %s for user %s', + ['exAppId' => 'test_app', 'userId' => 'alice', 'reason' => 'invalid_secret'], + ); + + self::assertFalse($this->service->validateExAppRequestToNC($this->createRequestForUser('test_app', 'alice', 'wrong_secret'))); + } + + public function testValidateLogsAnUnknownExApp(): void { + $this->exAppService->method('getExApp')->with('unknown_app')->willReturn(null); + $this->throttler->method('sleepDelayOrThrowOnMax')->willReturn(0); + $this->throttler->expects(self::once())->method('registerAttempt'); + $this->auditLogger->expects(self::once())->method('log')->with( + Operation::ExAppAuthFailed, + 'Failed AppAPI authentication of ExApp %s for user %s', + ['exAppId' => 'unknown_app', 'userId' => 'alice', 'reason' => 'unknown_ex_app'], + ); + + self::assertFalse($this->service->validateExAppRequestToNC($this->createRequestForUser('unknown_app', 'alice', 'any_secret'))); + } + + public function testValidateDoesNotLogARequestWithoutAnExAppId(): void { + $this->throttler->method('sleepDelayOrThrowOnMax')->willReturn(0); + $this->auditLogger->expects(self::never())->method('log'); + + self::assertFalse($this->service->validateExAppRequestToNC($this->createRequestForUser('', 'alice', 'any_secret'))); + } } diff --git a/tests/php/Service/DaemonConfigServiceTest.php b/tests/php/Service/DaemonConfigServiceTest.php index 8c6b0e240..a927b2283 100644 --- a/tests/php/Service/DaemonConfigServiceTest.php +++ b/tests/php/Service/DaemonConfigServiceTest.php @@ -9,17 +9,28 @@ namespace OCA\AppAPI\Tests\php\Service; +use OCA\AppAPI\AuditLog\AuditLogger; +use OCA\AppAPI\AuditLog\Operation; use OCA\AppAPI\Db\DaemonConfig; use OCA\AppAPI\Db\DaemonConfigMapper; use OCA\AppAPI\Service\DaemonConfigService; use OCA\AppAPI\Service\ExAppService; +use OCP\DB\Exception; use OCP\Security\ICrypto; use PHPUnit\Framework\Attributes\DataProvider; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; class DaemonConfigServiceTest extends TestCase { + private AuditLogger&MockObject $auditLogger; + + protected function setUp(): void { + parent::setUp(); + $this->auditLogger = $this->createMock(AuditLogger::class); + } + public static function resolveImageRegistryProvider(): array { $mirror = ['from' => 'ghcr.io', 'to' => 'registry.example.com']; return [ @@ -104,6 +115,7 @@ private function createService(bool $expectUpdate = true): DaemonConfigService { $mapper, $this->createMock(ExAppService::class), $this->createMock(ICrypto::class), + $this->auditLogger, ); } @@ -133,6 +145,7 @@ public static function unusableRegistryMapProvider(): array { #[DataProvider('unusableRegistryMapProvider')] public function testAddDockerRegistryRejectsAnUnusableMap(array $registryMap): void { + $this->auditLogger->expects(self::never())->method('log'); $daemonConfig = new DaemonConfig(['deploy_config' => ['registries' => []]]); $result = $this->createService(expectUpdate: false)->addDockerRegistry($daemonConfig, $registryMap); @@ -142,6 +155,7 @@ public function testAddDockerRegistryRejectsAnUnusableMap(array $registryMap): v } public function testAddDockerRegistryRejectsADuplicateSource(): void { + $this->auditLogger->expects(self::never())->method('log'); $daemonConfig = new DaemonConfig(['deploy_config' => ['registries' => ['junk', ['from' => 'ghcr.io', 'to' => 'local']]]]); $result = $this->createService(expectUpdate: false) @@ -163,7 +177,12 @@ public function testAddDockerRegistryIgnoresAnUnusableStoredEntryOfTheSameSource } public function testAddDockerRegistryStoresNormalisedSourceAndTarget(): void { - $daemonConfig = new DaemonConfig(['deploy_config' => []]); + $daemonConfig = new DaemonConfig(['name' => 'harp_proxy', 'deploy_config' => []]); + $this->auditLogger->expects(self::once())->method('log')->with( + Operation::DaemonRegistryAdded, + 'Docker registry mapping was added to deploy daemon %s: %s to %s', + ['daemonName' => 'harp_proxy', 'from' => 'ghcr.io', 'to' => 'registry.example.com'], + ); $result = $this->createService() ->addDockerRegistry($daemonConfig, ['from' => ' ghcr.io/ ', 'to' => 'registry.example.com//', 'extra' => 'x']); @@ -182,6 +201,7 @@ public function testAddDockerRegistryStoresLocalWithoutATrailingSlash(): void { } public function testRemoveDockerRegistryWithoutAnyMapping(): void { + $this->auditLogger->expects(self::never())->method('log'); $daemonConfig = new DaemonConfig(['deploy_config' => ['net' => 'host']]); $result = $this->createService(expectUpdate: false) @@ -194,7 +214,12 @@ public function testRemoveDockerRegistryKeepsAList(): void { $service = $this->createService(); $first = ['from' => 'docker.io', 'to' => 'hub.example.com']; $second = ['from' => 'ghcr.io', 'to' => 'registry.example.com']; - $daemonConfig = new DaemonConfig(['deploy_config' => ['registries' => [$first, $second]]]); + $daemonConfig = new DaemonConfig(['name' => 'harp_proxy', 'deploy_config' => ['registries' => [$first, $second]]]); + $this->auditLogger->expects(self::once())->method('log')->with( + Operation::DaemonRegistryRemoved, + 'Docker registry mapping was removed from deploy daemon %s: %s to %s', + ['daemonName' => 'harp_proxy', 'from' => 'docker.io', 'to' => 'hub.example.com'], + ); $result = $service->removeDockerRegistry($daemonConfig, $first); @@ -219,6 +244,7 @@ private function createServiceWithLogger(LoggerInterface $logger): DaemonConfigS $this->createMock(DaemonConfigMapper::class), $this->createMock(ExAppService::class), $this->createMock(ICrypto::class), + $this->auditLogger, ); } @@ -281,4 +307,118 @@ public function testValidateNewSecretDoesNotWarnAboutAGeneratedSecret(): void { self::assertNull($result); } + + private function createServiceWithMapper(DaemonConfigMapper $mapper): DaemonConfigService { + $crypto = $this->createMock(ICrypto::class); + $crypto->method('encrypt')->willReturnCallback(static fn (string $value): string => 'encrypted:' . $value); + return new DaemonConfigService( + $this->createMock(LoggerInterface::class), + $mapper, + $this->createMock(ExAppService::class), + $crypto, + $this->auditLogger, + ); + } + + private static function harpRegisterParams(): array { + return [ + 'name' => 'harp_proxy', + 'display_name' => 'HaRP Proxy', + 'accepts_deploy_id' => 'docker-install', + 'protocol' => 'http', + 'host' => 'appapi-harp:8780', + 'deploy_config' => [ + 'net' => 'host', + 'nextcloud_url' => 'http://nextcloud.local/', + 'haproxy_password' => 'a_secret_shared_key', + 'harp' => ['frp_address' => 'appapi-harp:8782'], + ], + ]; + } + + public function testRegisterDaemonConfigLogsTheDaemonWithoutTheSecret(): void { + $mapper = $this->createMock(DaemonConfigMapper::class); + $mapper->expects(self::once())->method('insert')->willReturnArgument(0); + $this->auditLogger->expects(self::once())->method('log')->with( + Operation::DaemonRegistered, + 'Deploy daemon %s was registered', + ['daemonName' => 'harp_proxy', 'acceptsDeployId' => 'docker-install', 'host' => 'appapi-harp:8780'], + ); + + $result = $this->createServiceWithMapper($mapper)->registerDaemonConfig(self::harpRegisterParams()); + + self::assertInstanceOf(DaemonConfig::class, $result); + } + + public function testRegisterDaemonConfigDoesNotLogAnInvalidDaemon(): void { + $mapper = $this->createMock(DaemonConfigMapper::class); + $mapper->expects(self::never())->method('insert'); + $this->auditLogger->expects(self::never())->method('log'); + + $params = self::harpRegisterParams(); + $params['host'] = 'http://appapi-harp:8780'; + + self::assertNull($this->createServiceWithMapper($mapper)->registerDaemonConfig($params)); + } + + public function testRegisterDaemonConfigDoesNotLogADatabaseError(): void { + $mapper = $this->createMock(DaemonConfigMapper::class); + $mapper->method('insert')->willThrowException(new Exception('duplicate name')); + $this->auditLogger->expects(self::never())->method('log'); + + self::assertNull($this->createServiceWithMapper($mapper)->registerDaemonConfig(self::harpRegisterParams())); + } + + public function testUnregisterDaemonConfigLogsTheDaemon(): void { + $mapper = $this->createMock(DaemonConfigMapper::class); + $mapper->expects(self::once())->method('delete')->willReturnArgument(0); + $this->auditLogger->expects(self::once())->method('log')->with( + Operation::DaemonUnregistered, + 'Deploy daemon %s was unregistered', + ['daemonName' => 'harp_proxy'], + ); + + $this->createServiceWithMapper($mapper)->unregisterDaemonConfig(new DaemonConfig(['name' => 'harp_proxy'])); + } + + public function testUnregisterDaemonConfigDoesNotLogADatabaseError(): void { + $mapper = $this->createMock(DaemonConfigMapper::class); + $mapper->method('delete')->willThrowException(new Exception('database is gone')); + $this->auditLogger->expects(self::never())->method('log'); + + self::assertNull($this->createServiceWithMapper($mapper)->unregisterDaemonConfig(new DaemonConfig(['name' => 'harp_proxy']))); + } + + public static function updateDaemonConfigSecretProvider(): array { + return [ + 'stored secret kept' => ['encrypted:old_secret', 'encrypted:old_secret', false], + 'new secret set' => ['encrypted:old_secret', 'encrypted:new_secret', true], + 'secret removed' => ['encrypted:old_secret', '', true], + 'no secret before or after' => [null, '', false], + ]; + } + + #[DataProvider('updateDaemonConfigSecretProvider')] + public function testUpdateDaemonConfigLogsOnlyWhetherTheSecretChanged(?string $storedSecret, string $newSecret, bool $secretChanged): void { + $mapper = $this->createMock(DaemonConfigMapper::class); + $mapper->expects(self::once())->method('update')->willReturnArgument(0); + $this->auditLogger->expects(self::once())->method('log')->with( + Operation::DaemonUpdated, + 'Deploy daemon %s was updated', + ['daemonName' => 'harp_proxy', 'host' => 'appapi-harp:8780', 'secretChanged' => $secretChanged], + ); + $previous = new DaemonConfig(['name' => 'harp_proxy', 'host' => 'appapi-harp:8780', 'deploy_config' => $storedSecret === null ? [] : ['haproxy_password' => $storedSecret]]); + $updated = new DaemonConfig(['name' => 'harp_proxy', 'host' => 'appapi-harp:8780', 'deploy_config' => ['haproxy_password' => $newSecret]]); + + $this->createServiceWithMapper($mapper)->updateDaemonConfig($updated, $previous); + } + + public function testUpdateDaemonConfigDoesNotLogADatabaseError(): void { + $mapper = $this->createMock(DaemonConfigMapper::class); + $mapper->method('update')->willThrowException(new Exception('database is gone')); + $this->auditLogger->expects(self::never())->method('log'); + $daemonConfig = new DaemonConfig(['name' => 'harp_proxy', 'deploy_config' => []]); + + self::assertNull($this->createServiceWithMapper($mapper)->updateDaemonConfig($daemonConfig, clone $daemonConfig)); + } } diff --git a/tests/php/Service/ExAppServiceAuditLogTest.php b/tests/php/Service/ExAppServiceAuditLogTest.php new file mode 100644 index 000000000..221af76b7 --- /dev/null +++ b/tests/php/Service/ExAppServiceAuditLogTest.php @@ -0,0 +1,170 @@ +createMock(ICacheFactory::class); + $cacheFactory->method('isAvailable')->willReturn(false); + $this->exAppMapper = $this->createMock(ExAppMapper::class); + $this->auditLogger = $this->createMock(AuditLogger::class); + + $this->service = new ExAppService( + $this->createMock(LoggerInterface::class), + $cacheFactory, + $this->createMock(IUserManager::class), + $this->createMock(ExAppFetcher::class), + $this->createMock(ExAppArchiveFetcher::class), + $this->exAppMapper, + $this->createMock(TopMenuService::class), + $this->createMock(InitialStateService::class), + $this->createMock(ScriptsService::class), + $this->createMock(StylesService::class), + $this->createMock(FilesActionsMenuService::class), + $this->createMock(TaskProcessingService::class), + $this->createMock(TalkBotsService::class), + $this->createMock(SettingsService::class), + $this->createMock(ExAppOccService::class), + $this->createMock(ExAppDeployOptionsService::class), + $this->createMock(ExAppSetupCheckService::class), + $this->createMock(IConfig::class), + $this->auditLogger, + ); + } + + private static function appInfo(): array { + return [ + 'id' => 'test_app', + 'version' => '1.0.0', + 'name' => 'Test App', + 'daemon_config_name' => 'harp_proxy', + 'port' => 23000, + 'secret' => 'the_secret_of_the_exapp', + ]; + } + + private static function exApp(): ExApp { + return new ExApp([ + 'appid' => 'test_app', + 'version' => '1.0.0', + 'name' => 'Test App', + 'daemon_config_name' => 'harp_proxy', + 'port' => 23000, + 'secret' => 'the_secret_of_the_exapp', + ]); + } + + public function testRegisterExAppLogsTheExAppWithoutItsSecret(): void { + $this->exAppMapper->expects(self::once())->method('insert')->willReturnArgument(0); + $this->exAppMapper->method('findByAppId')->willReturn(self::exApp()); + $this->auditLogger->expects(self::once())->method('log')->with( + Operation::ExAppRegistered, + 'ExApp %s (version %s) was registered on deploy daemon %s', + ['appId' => 'test_app', 'version' => '1.0.0', 'daemonName' => 'harp_proxy', 'port' => 23000], + ); + + self::assertInstanceOf(ExApp::class, $this->service->registerExApp(self::appInfo())); + } + + public function testRegisterExAppDoesNotLogADatabaseError(): void { + $this->exAppMapper->method('insert')->willThrowException(new Exception('duplicate appid')); + $this->auditLogger->expects(self::never())->method('log'); + + self::assertNull($this->service->registerExApp(self::appInfo())); + } + + public function testUnregisterExAppLogsTheExApp(): void { + $this->exAppMapper->method('findAll')->willReturn([self::exApp()]); + $this->exAppMapper->expects(self::once())->method('deleteExApp')->with('test_app')->willReturn(1); + $this->auditLogger->expects(self::once())->method('log')->with( + Operation::ExAppUnregistered, + 'ExApp %s was unregistered', + ['appId' => 'test_app'], + ); + + self::assertTrue($this->service->unregisterExApp('test_app')); + } + + public function testUnregisterExAppDoesNotLogWhenNothingWasDeleted(): void { + $this->exAppMapper->method('findAll')->willReturn([self::exApp()]); + $this->exAppMapper->method('deleteExApp')->willReturn(0); + $this->auditLogger->expects(self::never())->method('log'); + + self::assertFalse($this->service->unregisterExApp('test_app')); + } + + public function testUnregisterExAppDoesNotLogAnUnknownExApp(): void { + $this->exAppMapper->method('findAll')->willReturn([]); + $this->exAppMapper->expects(self::never())->method('deleteExApp'); + $this->auditLogger->expects(self::never())->method('log'); + + self::assertFalse($this->service->unregisterExApp('test_app')); + } + + public function testUpdateExAppInfoLogsBothVersions(): void { + $this->exAppMapper->expects(self::once())->method('updateExApp'); + $this->auditLogger->expects(self::once())->method('log')->with( + Operation::ExAppUpdated, + 'ExApp %s was updated from version %s to %s', + ['appId' => 'test_app', 'oldVersion' => '1.0.0', 'newVersion' => '1.1.0'], + ); + + self::assertTrue($this->service->updateExAppInfo(self::exApp(), ['version' => '1.1.0', 'name' => 'Test App'])); + } + + public function testUpdateExAppInfoDoesNotLogADatabaseError(): void { + $this->exAppMapper->method('updateExApp')->willThrowException(new Exception('database is gone')); + $this->auditLogger->expects(self::never())->method('log'); + + self::assertFalse($this->service->updateExAppInfo(self::exApp(), ['version' => '1.1.0', 'name' => 'Test App'])); + } + + public function testLogExAppDataDeleted(): void { + $this->auditLogger->expects(self::once())->method('log')->with( + Operation::ExAppDataDeleted, + 'Data of ExApp %s was deleted from deploy daemon %s', + ['appId' => 'test_app', 'daemonName' => 'harp_proxy'], + ); + + $this->service->logExAppDataDeleted(self::exApp()); + } +} diff --git a/tests/php/Service/ExAppServiceGetAppInfoTest.php b/tests/php/Service/ExAppServiceGetAppInfoTest.php index 8e052df8c..f68454db3 100644 --- a/tests/php/Service/ExAppServiceGetAppInfoTest.php +++ b/tests/php/Service/ExAppServiceGetAppInfoTest.php @@ -9,6 +9,7 @@ namespace OCA\AppAPI\Tests\php\Service; +use OCA\AppAPI\AuditLog\AuditLogger; use OCA\AppAPI\Db\ExAppMapper; use OCA\AppAPI\Fetcher\ExAppArchiveFetcher; use OCA\AppAPI\Fetcher\ExAppFetcher; @@ -65,6 +66,7 @@ protected function setUp(): void { $this->createMock(ExAppDeployOptionsService::class), $this->createMock(ExAppSetupCheckService::class), $this->createMock(IConfig::class), + $this->createMock(AuditLogger::class), ); }