refactor: make setEngine() atomic and remove swallowed exceptions

- Reorder AEngineHandler::setEngine() to save policy before persisting
  engine to appConfig; if policy save fails, engine is not changed
- Remove try/catch from configureIdentifyMethodsForEngine() so failures
  propagate instead of being logged and ignored
- Remove try/catch from AbstractIdentifyMethod::getRuntimeConfigInt() so
  programming errors (unknown policy keys) propagate rather than
  silently returning defaults

Signed-off-by: Vitor Mattos <[email protected]>

Author: vitormattos

lib/Handler/CertificateEngine/AEngineHandler.php

@@ -19,6 +19,9 @@
 use OCA\Libresign\Service\CertificatePolicyService;
 use OCA\Libresign\Service\Crl\CrlDistributionPointsExtractor;
 use OCA\Libresign\Service\Crl\CrlRevocationChecker;
+use OCA\Libresign\Service\Policy\PolicyService;
+use OCA\Libresign\Service\Policy\Provider\ExpirationRules\ExpirationRulesPolicy;
+use OCA\Libresign\Service\Policy\Provider\IdentifyMethods\IdentifyMethodsPolicy;
 use OCP\Files\AppData\IAppDataFactory;
 use OCP\Files\IAppData;
 use OCP\Files\SimpleFS\ISimpleFolder;
@@ -85,6 +88,7 @@ public function __construct(
 		protected CertificatePolicyService $certificatePolicyService,
 		protected IURLGenerator $urlGenerator,
 		protected CaIdentifierService $caIdentifierService,
+		protected PolicyService $policyService,
 		protected LoggerInterface $logger,
 		private CrlRevocationChecker $crlRevocationChecker,
 	) {
@@ -276,9 +280,9 @@ public function translateToLong($name): string {
 
 	#[\Override]
 	public function setEngine(string $engine): void {
+		$this->configureIdentifyMethodsForEngine($engine);
 		$this->appConfig->setValueString(Application::APP_ID, 'certificate_engine', $engine);
 		$this->engine = $engine;
-		$this->configureIdentifyMethodsForEngine($engine);
 	}
 
 	/**
@@ -305,7 +309,7 @@ private function configureIdentifyMethodsForEngine(string $engine): void {
 			'enabled' => true,
 			'mandatory' => true,
 		]];
-		$this->appConfig->setValueArray(Application::APP_ID, 'identify_methods', $config);
+		$this->policyService->saveSystem(IdentifyMethodsPolicy::KEY, $config);
 	}
 
 	#[\Override]
@@ -477,7 +481,7 @@ public function getLeafExpiryInDays(): int {
 		if ($this->leafExpiryOverrideInDays !== null) {
 			return $this->leafExpiryOverrideInDays;
 		}
-		$exp = $this->appConfig->getValueInt(Application::APP_ID, 'expiry_in_days', 365);
+		$exp = (int)$this->policyService->resolve(ExpirationRulesPolicy::KEY_EXPIRY_IN_DAYS)->getEffectiveValue();
 		return $exp > 0 ? $exp : 365;
 	}
 

lib/Service/IdentifyMethod/AbstractIdentifyMethod.php

@@ -11,13 +11,13 @@
 use DateTime;
 use InvalidArgumentException;
 use OC\AppFramework\Http as AppFrameworkHttp;
-use OCA\Libresign\AppInfo\Application;
 use OCA\Libresign\Db\IdentifyMethod;
 use OCA\Libresign\Enum\FileStatus;
 use OCA\Libresign\Events\SendSignNotificationEvent;
 use OCA\Libresign\Exception\LibresignException;
 use OCA\Libresign\Helper\JSActions;
 use OCA\Libresign\Service\IdentifyMethod\SignatureMethod\AbstractSignatureMethod;
+use OCA\Libresign\Service\Policy\Provider\ExpirationRules\ExpirationRulesPolicy;
 use OCA\Libresign\Service\SessionService;
 use OCA\Libresign\Vendor\Wobeto\EmailBlur\Blur;
 use OCP\Files\NotFoundException;
@@ -216,7 +216,7 @@ protected function throwIfFileNotFound(): void {
 	}
 
 	protected function throwIfMaximumValidityExpired(): void {
-		$maximumValidity = (int)$this->identifyService->getAppConfig()->getValueInt(Application::APP_ID, 'maximum_validity', SessionService::NO_MAXIMUM_VALIDITY);
+		$maximumValidity = $this->getRuntimeConfigInt(ExpirationRulesPolicy::KEY_MAXIMUM_VALIDITY, SessionService::NO_MAXIMUM_VALIDITY);
 		if ($maximumValidity <= 0) {
 			return;
 		}
@@ -247,11 +247,11 @@ protected function throwIfInvalidToken(): void {
 
 	protected function renewSession(): void {
 		$this->identifyService->getSessionService()->setIdentifyMethodId($this->getEntity()->getId());
-		$renewalInterval = $this->getRuntimeConfigInt('renewal_interval', SessionService::NO_RENEWAL_INTERVAL);
+		$renewalInterval = $this->getRuntimeConfigInt(ExpirationRulesPolicy::KEY_RENEWAL_INTERVAL, SessionService::NO_RENEWAL_INTERVAL);
 		if ($renewalInterval <= 0) {
 			return;
 		}
-		$this->identifyService->getSessionService()->resetDurationOfSignPage();
+		$this->identifyService->getSessionService()->resetDurationOfSignPage($renewalInterval);
 	}
 
 	protected function updateIdentifiedAt(): void {
@@ -265,7 +265,7 @@ protected function updateIdentifiedAt(): void {
 	}
 
 	protected function throwIfRenewalIntervalExpired(): void {
-		$renewalInterval = $this->getRuntimeConfigInt('renewal_interval', SessionService::NO_RENEWAL_INTERVAL);
+		$renewalInterval = $this->getRuntimeConfigInt(ExpirationRulesPolicy::KEY_RENEWAL_INTERVAL, SessionService::NO_RENEWAL_INTERVAL);
 		if ($renewalInterval <= 0) {
 			return;
 		}
@@ -318,9 +318,9 @@ private function getRenewAction(): int {
 	}
 
 	private function getRuntimeConfigInt(string $key, int $default): int {
-		$appConfig = $this->identifyService->getAppConfig();
-		$appConfig->clearCache(true);
-		return (int)$appConfig->getValueInt(Application::APP_ID, $key, $default);
+		$resolved = $this->identifyService->getPolicyService()->resolve($key);
+		$value = $resolved->getEffectiveValue();
+		return $value !== null ? (int)$value : $default;
 	}
 
 	protected function throwIfAlreadySigned(): void {

tests/php/Unit/Handler/CertificateEngine/AEngineHandlerTest.php

@@ -68,6 +68,7 @@ private function getInstance(): OpenSslHandler {
 			$this->urlGenerator,
 			\OCP\Server::get(\OCA\Libresign\Service\SerialNumberService::class),
 			$this->caIdentifierService,
+			\OCP\Server::get(\OCA\Libresign\Service\Policy\PolicyService::class),
 			$this->logger,
 			\OCP\Server::get(\OCA\Libresign\Db\CrlMapper::class),
 			$this->subjectAlternativeNameService,
@@ -116,6 +117,7 @@ public function testSetEngineConfiguresIdentifyMethodsForNoneEngine(
 			'name' => 'account',
 			'enabled' => true,
 			'mandatory' => true,
+			'signatureMethods' => [],
 		]];
 
 		$this->assertEquals(