diff --git a/appinfo/info.xml b/appinfo/info.xml index ec1c2edd..29ccd9f0 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -38,7 +38,7 @@ to join us in shaping a more versatile, stable, and secure app landscape. *Your insights, suggestions, and contributions are invaluable to us.* ]]> - 32.0.0-dev.4 + 32.0.0-dev.5 agpl Andrey Borysenko Alexander Piskun diff --git a/lib/Controller/PreferencesController.php b/lib/Controller/PreferencesController.php index 1c957e58..d120dda8 100644 --- a/lib/Controller/PreferencesController.php +++ b/lib/Controller/PreferencesController.php @@ -43,13 +43,13 @@ public function __construct( #[AppAPIAuth] #[PublicPage] #[NoCSRFRequired] - public function setUserConfigValue(string $configKey, mixed $configValue): DataResponse { + public function setUserConfigValue(string $configKey, mixed $configValue, ?int $sensitive = null): DataResponse { if ($configKey === '') { throw new OCSBadRequestException('Config key cannot be empty'); } $userId = $this->userSession->getUser()->getUID(); $appId = $this->request->getHeader('EX-APP-ID'); - $result = $this->exAppPreferenceService->setUserConfigValue($userId, $appId, $configKey, $configValue); + $result = $this->exAppPreferenceService->setUserConfigValue($userId, $appId, $configKey, $configValue, $sensitive); if ($result instanceof ExAppPreference) { return new DataResponse($result, Http::STATUS_OK); } diff --git a/lib/Db/ExAppPreference.php b/lib/Db/ExAppPreference.php index cc898e79..72ce5331 100644 --- a/lib/Db/ExAppPreference.php +++ b/lib/Db/ExAppPreference.php @@ -21,16 +21,19 @@ * @method string getAppid() * @method string getConfigkey() * @method string getConfigvalue() + * @method int getSensitive() * @method void setUserid(string $userid) * @method void setAppid(string $appid) * @method void setConfigkey(string $configkey) * @method void setConfigvalue(string $configvalue) + * @method void setSensitive(int $sensitive) */ class ExAppPreference extends Entity implements JsonSerializable { protected $userid; protected $appid; protected $configkey; protected $configvalue; + protected $sensitive; /** * @param array $params @@ -40,6 +43,7 @@ public function __construct(array $params = []) { $this->addType('appid', 'string'); $this->addType('configkey', 'string'); $this->addType('configvalue', 'string'); + $this->addType('sensitive', 'int'); if (isset($params['id'])) { $this->setId($params['id']); @@ -56,6 +60,9 @@ public function __construct(array $params = []) { if (isset($params['configvalue'])) { $this->setConfigvalue($params['configvalue']); } + if (isset($params['sensitive'])) { + $this->setSensitive($params['sensitive']); + } } public function jsonSerialize(): array { @@ -65,6 +72,7 @@ public function jsonSerialize(): array { 'appid' => $this->getAppid(), 'configkey' => $this->getConfigkey(), 'configvalue' => $this->getConfigvalue(), + 'sensitive' => $this->getSensitive(), ]; } } diff --git a/lib/Db/UI/SettingsForm.php b/lib/Db/UI/SettingsForm.php index d8a88ac1..0b0214d8 100644 --- a/lib/Db/UI/SettingsForm.php +++ b/lib/Db/UI/SettingsForm.php @@ -47,6 +47,16 @@ public function __construct(array $params = []) { } } + public function getSchemaField(string $fieldId): ?array { + $scheme = $this->getScheme(); + foreach ($scheme['fields'] as $field) { + if ($field['id'] === $fieldId) { + return $field; + } + } + return null; + } + public function jsonSerialize(): array { return [ 'id' => $this->getId(), diff --git a/lib/Listener/DeclarativeSettings/GetValueListener.php b/lib/Listener/DeclarativeSettings/GetValueListener.php index 3b6deda5..ade1d57b 100644 --- a/lib/Listener/DeclarativeSettings/GetValueListener.php +++ b/lib/Listener/DeclarativeSettings/GetValueListener.php @@ -14,17 +14,21 @@ use OCA\AppAPI\Service\UI\SettingsService; use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventListener; +use OCP\Security\ICrypto; use OCP\Settings\DeclarativeSettingsTypes; use OCP\Settings\Events\DeclarativeSettingsGetValueEvent; +use Psr\Log\LoggerInterface; /** * @template-implements IEventListener */ class GetValueListener implements IEventListener { public function __construct( - private readonly SettingsService $service, + private readonly SettingsService $service, private readonly ExAppPreferenceService $preferenceService, - private readonly ExAppConfigService $configService, + private readonly ExAppConfigService $configService, + private readonly ICrypto $crypto, + private readonly LoggerInterface $logger, ) { } @@ -38,9 +42,20 @@ public function handle(Event $event): void { return; } $formSchema = $settingsForm->getScheme(); + $field = $settingsForm->getSchemaField($event->getFieldId()); + $isSensitive = isset($field['sensitive']) && $field['sensitive'] === true; if ($formSchema['section_type'] === DeclarativeSettingsTypes::SECTION_TYPE_ADMIN) { $existingValue = $this->configService->getAppConfig($event->getApp(), $event->getFieldId()); if (!empty($existingValue)) { + if ($isSensitive) { + try { + $decryptedValue = $this->crypto->decrypt($existingValue->getConfigvalue()); + $existingValue->setConfigvalue($decryptedValue); + } catch (\Exception $e) { + $this->logger->warning(sprintf('Failed to decrypt declarative setting for app %s, field %s', $event->getApp(), $event->getFieldId()), ['exception' => $e]); + $existingValue->setConfigvalue(''); + } + } $event->setValue($existingValue->getConfigvalue()); return; } @@ -51,6 +66,15 @@ public function handle(Event $event): void { [$event->getFieldId()], ); if (!empty($existingValue)) { + if ($isSensitive) { + try { + $decryptedValue = $this->crypto->decrypt($existingValue[0]['configvalue']); + $existingValue[0]['configvalue'] = $decryptedValue; + } catch (\Exception $e) { + $this->logger->warning('Failed to decrypt declarative setting for app ' . $event->getApp() . ', field ' . $event->getFieldId(), ['exception' => $e]); + $existingValue[0]['configvalue'] = ''; + } + } $event->setValue($existingValue[0]['configvalue']); return; } diff --git a/lib/Listener/DeclarativeSettings/SetValueListener.php b/lib/Listener/DeclarativeSettings/SetValueListener.php index 626b60a9..5972c521 100644 --- a/lib/Listener/DeclarativeSettings/SetValueListener.php +++ b/lib/Listener/DeclarativeSettings/SetValueListener.php @@ -14,17 +14,21 @@ use OCA\AppAPI\Service\UI\SettingsService; use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventListener; +use OCP\Security\ICrypto; use OCP\Settings\DeclarativeSettingsTypes; use OCP\Settings\Events\DeclarativeSettingsSetValueEvent; +use Psr\Log\LoggerInterface; /** * @template-implements IEventListener */ class SetValueListener implements IEventListener { public function __construct( - private readonly SettingsService $service, + private readonly SettingsService $service, private readonly ExAppPreferenceService $preferenceService, - private readonly ExAppConfigService $configService, + private readonly ExAppConfigService $configService, + private readonly ICrypto $crypto, + private readonly LoggerInterface $logger, ) { } @@ -38,11 +42,25 @@ public function handle(Event $event): void { return; } $formSchema = $settingsForm->getScheme(); + $field = $settingsForm->getSchemaField($event->getFieldId()); + $isSensitive = isset($field['sensitive']) && $field['sensitive'] === true; + $value = $event->getValue(); + if ($isSensitive) { + try { + $value = $this->crypto->encrypt($value); + } catch (\Exception $e) { + $this->logger->warning( + sprintf('Failed to encrypt sensitive value for app %s, field %s', $event->getApp(), $event->getFieldId()), + ['exception' => $e, 'app' => $event->getApp()] + ); + return; + } + } if ($formSchema['section_type'] === DeclarativeSettingsTypes::SECTION_TYPE_ADMIN) { - $this->configService->setAppConfigValue($event->getApp(), $event->getFieldId(), $event->getValue()); + $this->configService->setAppConfigValue($event->getApp(), $event->getFieldId(), $value); } else { $this->preferenceService->setUserConfigValue( - $event->getUser()->getUID(), $event->getApp(), $event->getFieldId(), $event->getValue() + $event->getUser()->getUID(), $event->getApp(), $event->getFieldId(), $value ); } } diff --git a/lib/Migration/Version032002Date20250527174907.php b/lib/Migration/Version032002Date20250527174907.php new file mode 100644 index 00000000..b20c0c67 --- /dev/null +++ b/lib/Migration/Version032002Date20250527174907.php @@ -0,0 +1,89 @@ +hasTable('preferences_ex')) { + $table = $schema->getTable('preferences_ex'); + + if (!$table->hasColumn('sensitive')) { + $table->addColumn('sensitive', Types::SMALLINT, [ + 'notnull' => true, + 'default' => 0, + ]); + } + } + + return $schema; + } + + /** + * @param IOutput $output + * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper` + * @param array $options + * + * @return null|ISchemaWrapper + */ + public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options) { + // encrypt appconfig_ex values that have sensitive flag set + + $qbSelect = $this->connection->getQueryBuilder(); + $qbSelect->select(['id', 'configvalue']) + ->from('appconfig_ex') + ->where($qbSelect->expr()->eq('sensitive', $qbSelect->createNamedParameter(1, Types::SMALLINT))); + $req = $qbSelect->executeQuery(); + + while ($row = $req->fetch()) { + $configValue = $row['configvalue']; + if (!empty($configValue)) { + try { + $encryptedValue = $this->crypto->encrypt($configValue); + $qbUpdate = $this->connection->getQueryBuilder(); + $qbUpdate->update('appconfig_ex') + ->set('configvalue', $qbUpdate->createNamedParameter($encryptedValue)) + ->where( + $qbUpdate->expr()->eq('id', $qbUpdate->createNamedParameter($row['id'], Types::INTEGER)) + ); + $qbUpdate->executeStatement(); + } catch (\Exception $e) { + $output->warning(sprintf('Failed to encrypt sensitive value for app config id %s: %s', $row['id'], $e->getMessage())); + } + } + } + + $req->closeCursor(); + return null; + } +} diff --git a/lib/Service/ExAppConfigService.php b/lib/Service/ExAppConfigService.php index 4da75d9e..096faa48 100644 --- a/lib/Service/ExAppConfigService.php +++ b/lib/Service/ExAppConfigService.php @@ -15,6 +15,7 @@ use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\DB\Exception; +use OCP\Security\ICrypto; use Psr\Log\LoggerInterface; /** @@ -25,15 +26,25 @@ class ExAppConfigService { public function __construct( private ExAppConfigMapper $mapper, private LoggerInterface $logger, + private ICrypto $crypto, ) { } public function getAppConfigValues(string $appId, array $configKeys): ?array { try { return array_map(function (ExAppConfig $exAppConfig) { + $value = $exAppConfig->getConfigvalue() ?? ''; + if ($value !== '' && $exAppConfig->getSensitive()) { + try { + $value = $this->crypto->decrypt($value); + } catch (\Exception $e) { + $this->logger->warning(sprintf('Failed to decrypt sensitive value for app %s, config key %s', $exAppConfig->getAppid(), $exAppConfig->getConfigkey()), ['exception' => $e]); + $value = ''; + } + } return [ 'configkey' => $exAppConfig->getConfigkey(), - 'configvalue' => $exAppConfig->getConfigvalue() ?? '', + 'configvalue' => $value, ]; }, $this->mapper->findByAppConfigKeys($appId, $configKeys)); } catch (Exception) { @@ -43,12 +54,22 @@ public function getAppConfigValues(string $appId, array $configKeys): ?array { public function setAppConfigValue(string $appId, string $configKey, mixed $configValue, ?int $sensitive = null): ?ExAppConfig { $appConfigEx = $this->getAppConfig($appId, $configKey); + if ($configValue !== '' && $sensitive) { + try { + $encryptedValue = $this->crypto->encrypt($configValue); + } catch (\Exception $e) { + $this->logger->error(sprintf('Failed to encrypt sensitive value for app %s, config key %s. Error: %s', $appId, $configKey, $e->getMessage()), ['exception' => $e]); + return null; + } + } else { + $encryptedValue = ''; + } if ($appConfigEx === null) { try { $appConfigEx = $this->mapper->insert(new ExAppConfig([ 'appid' => $appId, 'configkey' => $configKey, - 'configvalue' => $configValue ?? '', + 'configvalue' => $sensitive ? $encryptedValue : $configValue ?? '', 'sensitive' => $sensitive ?? 0, ])); } catch (Exception $e) { @@ -56,7 +77,7 @@ public function setAppConfigValue(string $appId, string $configKey, mixed $confi return null; } } else { - $appConfigEx->setConfigvalue($configValue); + $appConfigEx->setConfigvalue($sensitive ? $encryptedValue : $configValue); if ($sensitive !== null) { $appConfigEx->setSensitive($sensitive); } @@ -65,6 +86,10 @@ public function setAppConfigValue(string $appId, string $configKey, mixed $confi return null; } } + if ($sensitive) { + // setting original unencrypted value for API + $appConfigEx->setConfigvalue($configValue); + } return $appConfigEx; } diff --git a/lib/Service/ExAppPreferenceService.php b/lib/Service/ExAppPreferenceService.php index 9fd39f2f..38923934 100644 --- a/lib/Service/ExAppPreferenceService.php +++ b/lib/Service/ExAppPreferenceService.php @@ -15,6 +15,7 @@ use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\DB\Exception; +use OCP\Security\ICrypto; use Psr\Log\LoggerInterface; /** @@ -25,48 +26,76 @@ class ExAppPreferenceService { public function __construct( private ExAppPreferenceMapper $mapper, private LoggerInterface $logger, + private ICrypto $crypto, ) { } - public function setUserConfigValue(string $userId, string $appId, string $configKey, mixed $configValue) { + public function setUserConfigValue(string $userId, string $appId, string $configKey, mixed $configValue, ?int $sensitive = null): ?ExAppPreference { try { $exAppPreference = $this->mapper->findByUserIdAppIdKey($userId, $appId, $configKey); } catch (DoesNotExistException|MultipleObjectsReturnedException|Exception) { $exAppPreference = null; } + if ($configValue !== '' && $sensitive) { + try { + $encryptedValue = $this->crypto->encrypt($configValue); + } catch (\Exception $e) { + $this->logger->error('Failed to encrypt sensitive value: ' . $e->getMessage(), ['exception' => $e]); + return null; + } + } else { + $encryptedValue = ''; + } if ($exAppPreference === null) { try { - return $this->mapper->insert(new ExAppPreference([ + $exAppPreference = $this->mapper->insert(new ExAppPreference([ 'userid' => $userId, 'appid' => $appId, 'configkey' => $configKey, - 'configvalue' => $configValue ?? '', + 'configvalue' => $sensitive ? $encryptedValue : $configValue ?? '', + 'sensitive' => $sensitive ?? 0, ])); } catch (Exception $e) { $this->logger->error('Error while inserting new config value: ' . $e->getMessage(), ['exception' => $e]); return null; } } else { - $exAppPreference->setConfigvalue($configValue); + $exAppPreference->setConfigvalue($sensitive ? $encryptedValue : $configValue); + if ($sensitive !== null) { + $exAppPreference->setSensitive($sensitive); + } try { if ($this->mapper->updateUserConfigValue($exAppPreference) !== 1) { $this->logger->error('Error while updating preferences_ex config value'); return null; } - return $exAppPreference; } catch (Exception $e) { $this->logger->error('Error while updating config value: ' . $e->getMessage(), ['exception' => $e]); return null; } } + if ($sensitive) { + // setting original unencrypted value for API + $exAppPreference->setConfigvalue($configValue); + } + return $exAppPreference; } public function getUserConfigValues(string $userId, string $appId, array $configKeys): ?array { try { return array_map(function (ExAppPreference $exAppPreference) { + $value = $exAppPreference->getConfigvalue() ?? ''; + if ($value !== '' && $exAppPreference->getSensitive()) { + try { + $value = $this->crypto->decrypt($value); + } catch (\Exception $e) { + $this->logger->warning(sprintf('Failed to decrypt sensitive value for user %s, app %s, config key %s', $exAppPreference->getUserid(), $exAppPreference->getAppid(), $exAppPreference->getConfigkey()), ['exception' => $e]); + $value = ''; + } + } return [ 'configkey' => $exAppPreference->getConfigkey(), - 'configvalue' => $exAppPreference->getConfigvalue() ?? '', + 'configvalue' => $value, ]; }, $this->mapper->findByUserIdAppIdKeys($userId, $appId, $configKeys)); } catch (Exception) {