diff --git a/appinfo/info.xml b/appinfo/info.xml index ca2079604..4ec6cee6e 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -37,7 +37,7 @@ Two options are available: See the [admin documentation](https://docs.nextcloud.com/server/latest/admin_manual/exapps_management/DeployConfigurations.html) for setup instructions. ]]> - 35.0.0-dev.0 + 35.0.0-dev.1 agpl Andrey Borysenko Alexander Piskun diff --git a/appinfo/routes.php b/appinfo/routes.php index 06d673f9c..0b208af9f 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -87,12 +87,16 @@ // ExApps actions ['name' => 'OCSExApp#setExAppEnabled', 'url' => '/api/v1/ex-app/{appId}/enabled', 'verb' => 'PUT'], - // appconfig_ex (app configuration) + // ExApp app configuration (backed by the server's IAppConfig / oc_appconfig). + // Intentionally kept as AppAPI endpoints: the server's own provisioning_api app-config + // routes require admin / delegated-admin authorization, which an ExApp request lacks. ['name' => 'AppConfig#setAppConfigValue', 'url' => '/api/v1/ex-app/config', 'verb' => 'POST'], ['name' => 'AppConfig#getAppConfigValues', 'url' => '/api/v1/ex-app/config/get-values', 'verb' => 'POST'], ['name' => 'AppConfig#deleteAppConfigValues', 'url' => '/api/v1/ex-app/config', 'verb' => 'DELETE'], - // preferences_ex (user-specific configuration) + // ExApp per-user preferences (backed by the server's IUserConfig / oc_preferences). + // Intentionally kept: the server's provisioning_api user-config routes are write-only + // (no read endpoint) and have no sensitive/encrypted-value support. ['name' => 'Preferences#setUserConfigValue', 'url' => '/api/v1/ex-app/preference', 'verb' => 'POST'], ['name' => 'Preferences#getUserConfigValues', 'url' => '/api/v1/ex-app/preference/get-values', 'verb' => 'POST'], ['name' => 'Preferences#deleteUserConfigValues', 'url' => '/api/v1/ex-app/preference', 'verb' => 'DELETE'], diff --git a/lib/Controller/AppConfigController.php b/lib/Controller/AppConfigController.php index a238855eb..02c0e02bb 100644 --- a/lib/Controller/AppConfigController.php +++ b/lib/Controller/AppConfigController.php @@ -76,7 +76,7 @@ public function deleteAppConfigValues(array $configKeys): DataResponse { throw new OCSBadRequestException('Error deleting app config values'); } if ($result === 0) { - throw new OCSNotFoundException('No appconfig_ex values deleted'); + throw new OCSNotFoundException('No app config values deleted'); } return new DataResponse($result, Http::STATUS_OK); } diff --git a/lib/Controller/PreferencesController.php b/lib/Controller/PreferencesController.php index 99074ef19..f157fcb99 100644 --- a/lib/Controller/PreferencesController.php +++ b/lib/Controller/PreferencesController.php @@ -81,7 +81,7 @@ public function deleteUserConfigValues(array $configKeys): DataResponse { throw new OCSBadRequestException('Failed to delete user config values'); } if ($result === 0) { - throw new OCSNotFoundException('No preferences_ex values deleted'); + throw new OCSNotFoundException('No user config values deleted'); } return new DataResponse($result, Http::STATUS_OK); } diff --git a/lib/Db/ExAppConfig.php b/lib/Db/ExAppConfig.php index 39550754f..57647f2bd 100644 --- a/lib/Db/ExAppConfig.php +++ b/lib/Db/ExAppConfig.php @@ -10,61 +10,67 @@ namespace OCA\AppAPI\Db; use JsonSerializable; -use OCP\AppFramework\Db\Entity; /** - * Class ExAppConfig + * App configuration value object. * - * @package OCA\AppAPI\Db + * Historically a database entity backed by the `appconfig_ex` table. ExApp config now lives + * in the server's standard `oc_appconfig` (via {@see \OCA\AppAPI\Service\ExAppConfigService}); + * this class remains as a plain serializable DTO for OCS responses and internal callers. * - * @method string getAppid() - * @method string getConfigkey() - * @method string getConfigvalue() - * @method int getSensitive() - * @method void setAppid(string $appId) - * @method void setConfigKey(string $configKey) - * @method void setConfigvalue(string $configValue) - * @method void setSensitive(int $sensitive) + * The `id` field has no surrogate key anymore (the server table uses a composite primary key); + * it is kept in the serialized shape as `0` for backwards compatibility and is unused. */ -class ExAppConfig extends Entity implements JsonSerializable { - protected $appid; - protected $configkey; - protected $configvalue; - protected $sensitive; +class ExAppConfig implements JsonSerializable { + private int $id; + private string $appid; + private string $configkey; + private string $configvalue; + private int $sensitive; - /** - * @param array $params - */ public function __construct(array $params = []) { - $this->addType('appid', 'string'); - $this->addType('configkey', 'string'); - $this->addType('configvalue', 'string'); - $this->addType('sensitive', 'integer'); + $this->id = isset($params['id']) ? (int)$params['id'] : 0; + $this->appid = (string)($params['appid'] ?? ''); + $this->configkey = (string)($params['configkey'] ?? ''); + $this->configvalue = (string)($params['configvalue'] ?? ''); + $this->sensitive = (int)($params['sensitive'] ?? 0); + } + + public function getId(): int { + return $this->id; + } + + public function getAppid(): string { + return $this->appid; + } + + public function getConfigkey(): string { + return $this->configkey; + } + + public function getConfigvalue(): string { + return $this->configvalue; + } + + public function setConfigvalue(string $configValue): void { + $this->configvalue = $configValue; + } + + public function getSensitive(): int { + return $this->sensitive; + } - if (isset($params['id'])) { - $this->setId($params['id']); - } - if (isset($params['appid'])) { - $this->setAppid($params['appid']); - } - if (isset($params['configkey'])) { - $this->setConfigkey($params['configkey']); - } - if (isset($params['configvalue'])) { - $this->setConfigvalue($params['configvalue']); - } - if (isset($params['sensitive'])) { - $this->setSensitive($params['sensitive']); - } + public function setSensitive(int $sensitive): void { + $this->sensitive = $sensitive; } public function jsonSerialize(): array { return [ - 'id' => $this->getId(), - 'appid' => $this->getAppid(), - 'configkey' => $this->getConfigkey(), - 'configvalue' => $this->getConfigvalue(), - 'sensitive' => $this->getSensitive(), + 'id' => $this->id, + 'appid' => $this->appid, + 'configkey' => $this->configkey, + 'configvalue' => $this->configvalue, + 'sensitive' => $this->sensitive, ]; } } diff --git a/lib/Db/ExAppConfigMapper.php b/lib/Db/ExAppConfigMapper.php deleted file mode 100644 index e4dab4c69..000000000 --- a/lib/Db/ExAppConfigMapper.php +++ /dev/null @@ -1,91 +0,0 @@ - - */ -class ExAppConfigMapper extends QBMapper { - public function __construct(IDBConnection $db) { - parent::__construct($db, 'appconfig_ex'); - } - - /** - * @throws Exception - */ - public function findAllByAppid(string $appId): array { - $qb = $this->db->getQueryBuilder(); - $qb->select('*') - ->from($this->tableName) - ->where($qb->expr()->eq('appid', $qb->createNamedParameter($appId, IQueryBuilder::PARAM_STR))); - return $this->findEntities($qb); - } - - /** - * @param string $appId - * @param string $configKey - * - * @throws DoesNotExistException if not found - * @throws MultipleObjectsReturnedException if more than one result - * @throws Exception - * - * @return ExAppConfig - */ - public function findByAppConfigKey(string $appId, string $configKey): Entity { - $qb = $this->db->getQueryBuilder(); - $qb->select('*') - ->from($this->tableName) - ->where( - $qb->expr()->eq('appid', $qb->createNamedParameter($appId, IQueryBuilder::PARAM_STR)), - $qb->expr()->eq('configkey', $qb->createNamedParameter($configKey, IQueryBuilder::PARAM_STR)) - ); - return $this->findEntity($qb); - } - - /** - * @param string $appId - * @param array $configKeys - * - * @throws Exception - * - * @return ExAppConfig[] - */ - public function findByAppConfigKeys(string $appId, array $configKeys): array { - $qb = $this->db->getQueryBuilder(); - $qb->select('*') - ->from($this->tableName) - ->where( - $qb->expr()->eq('appid', $qb->createNamedParameter($appId, IQueryBuilder::PARAM_STR)), - $qb->expr()->in('configkey', $qb->createNamedParameter($configKeys, IQueryBuilder::PARAM_STR_ARRAY), IQueryBuilder::PARAM_STR) - ); - return $this->findEntities($qb); - } - - /** - * @throws Exception - */ - public function deleteByAppidConfigkeys(string $appId, array $configKeys): int { - $qb = $this->db->getQueryBuilder(); - return $qb->delete($this->tableName) - ->where( - $qb->expr()->eq('appid', $qb->createNamedParameter($appId, IQueryBuilder::PARAM_STR)), - $qb->expr()->in('configkey', $qb->createNamedParameter($configKeys, IQueryBuilder::PARAM_STR_ARRAY), IQueryBuilder::PARAM_STR) - ) - ->executeStatement(); - } -} diff --git a/lib/Db/ExAppPreference.php b/lib/Db/ExAppPreference.php index 7c3433007..944e25251 100644 --- a/lib/Db/ExAppPreference.php +++ b/lib/Db/ExAppPreference.php @@ -10,69 +10,74 @@ namespace OCA\AppAPI\Db; use JsonSerializable; -use OCP\AppFramework\Db\Entity; /** - * Class ExAppPreference + * App per-user preference value object. * - * @package OCA\AppAPI\Db + * Historically a database entity backed by the `preferences_ex` table. ExApp preferences now + * live in the server's standard `oc_preferences` (via {@see \OCA\AppAPI\Service\ExAppPreferenceService}); + * this class remains as a plain serializable DTO for OCS responses and internal callers. * - * @method string getUserid() - * @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) + * The `id` field has no surrogate key anymore (the server table uses a composite primary key); + * it is kept in the serialized shape as `0` for backwards compatibility and is unused. */ -class ExAppPreference extends Entity implements JsonSerializable { - protected $userid; - protected $appid; - protected $configkey; - protected $configvalue; - protected $sensitive; +class ExAppPreference implements JsonSerializable { + private int $id; + private string $userid; + private string $appid; + private string $configkey; + private string $configvalue; + private int $sensitive; - /** - * @param array $params - */ public function __construct(array $params = []) { - $this->addType('userid', 'string'); - $this->addType('appid', 'string'); - $this->addType('configkey', 'string'); - $this->addType('configvalue', 'string'); - $this->addType('sensitive', 'integer'); + $this->id = isset($params['id']) ? (int)$params['id'] : 0; + $this->userid = (string)($params['userid'] ?? ''); + $this->appid = (string)($params['appid'] ?? ''); + $this->configkey = (string)($params['configkey'] ?? ''); + $this->configvalue = (string)($params['configvalue'] ?? ''); + $this->sensitive = (int)($params['sensitive'] ?? 0); + } + + public function getId(): int { + return $this->id; + } + + public function getUserid(): string { + return $this->userid; + } + + public function getAppid(): string { + return $this->appid; + } + + public function getConfigkey(): string { + return $this->configkey; + } + + public function getConfigvalue(): string { + return $this->configvalue; + } + + public function setConfigvalue(string $configValue): void { + $this->configvalue = $configValue; + } + + public function getSensitive(): int { + return $this->sensitive; + } - if (isset($params['id'])) { - $this->setId($params['id']); - } - if (isset($params['userid'])) { - $this->setUserid($params['userid']); - } - if (isset($params['appid'])) { - $this->setAppid($params['appid']); - } - if (isset($params['configkey'])) { - $this->setConfigkey($params['configkey']); - } - if (isset($params['configvalue'])) { - $this->setConfigvalue($params['configvalue']); - } - if (isset($params['sensitive'])) { - $this->setSensitive($params['sensitive']); - } + public function setSensitive(int $sensitive): void { + $this->sensitive = $sensitive; } public function jsonSerialize(): array { return [ - 'id' => $this->getId(), - 'user_id' => $this->getUserid(), - 'appid' => $this->getAppid(), - 'configkey' => $this->getConfigkey(), - 'configvalue' => $this->getConfigvalue(), - 'sensitive' => $this->getSensitive(), + 'id' => $this->id, + 'user_id' => $this->userid, + 'appid' => $this->appid, + 'configkey' => $this->configkey, + 'configvalue' => $this->configvalue, + 'sensitive' => $this->sensitive, ]; } } diff --git a/lib/Db/ExAppPreferenceMapper.php b/lib/Db/ExAppPreferenceMapper.php deleted file mode 100644 index 3c41e0ddd..000000000 --- a/lib/Db/ExAppPreferenceMapper.php +++ /dev/null @@ -1,99 +0,0 @@ - - */ -class ExAppPreferenceMapper extends QBMapper { - public function __construct(IDBConnection $db) { - parent::__construct($db, 'preferences_ex'); - } - - /** - * @param string $userId - * @param string $appId - * @param string $configKey - * - * @throws DoesNotExistException - * @throws Exception - * @throws MultipleObjectsReturnedException - * - * @return ExAppPreference|null - */ - public function findByUserIdAppIdKey(string $userId, string $appId, string $configKey): ?ExAppPreference { - $qb = $this->db->getQueryBuilder(); - $qb->select('*') - ->from($this->tableName) - ->where( - $qb->expr()->eq('userid', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR)), - $qb->expr()->eq('appid', $qb->createNamedParameter($appId, IQueryBuilder::PARAM_STR)), - $qb->expr()->eq('configkey', $qb->createNamedParameter($configKey, IQueryBuilder::PARAM_STR)) - ); - return $this->findEntity($qb); - } - - /** - * @param string $userId - * @param string $appId - * @param array $configKeys - * - * @throws Exception - * - * @return ExAppPreference[] - */ - public function findByUserIdAppIdKeys(string $userId, string $appId, array $configKeys): array { - $qb = $this->db->getQueryBuilder(); - $qb->select('*') - ->from($this->tableName) - ->where( - $qb->expr()->eq('userid', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR)), - $qb->expr()->eq('appid', $qb->createNamedParameter($appId, IQueryBuilder::PARAM_STR)), - $qb->expr()->in('configkey', $qb->createNamedParameter($configKeys, IQueryBuilder::PARAM_STR_ARRAY)) - ); - return $this->findEntities($qb); - } - - /** - * @throws Exception - */ - public function updateUserConfigValue(ExAppPreference $exAppPreference): int { - $qb = $this->db->getQueryBuilder(); - $qb->update($this->tableName) - ->set('configvalue', $qb->createNamedParameter($exAppPreference->getConfigvalue(), IQueryBuilder::PARAM_STR)) - ->where( - $qb->expr()->eq('userid', $qb->createNamedParameter($exAppPreference->getUserid(), IQueryBuilder::PARAM_STR)), - $qb->expr()->eq('appid', $qb->createNamedParameter($exAppPreference->getAppid(), IQueryBuilder::PARAM_STR)), - $qb->expr()->eq('configkey', $qb->createNamedParameter($exAppPreference->getConfigkey(), IQueryBuilder::PARAM_STR)) - ); - return $qb->executeStatement(); - } - - /** - * @throws Exception - */ - public function deleteUserConfigValues(array $configKeys, string $userId, string $appId): int { - $qb = $this->db->getQueryBuilder(); - $qb->delete($this->tableName) - ->where( - $qb->expr()->eq('userid', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR)), - $qb->expr()->eq('appid', $qb->createNamedParameter($appId, IQueryBuilder::PARAM_STR)), - $qb->expr()->in('configkey', $qb->createNamedParameter($configKeys, IQueryBuilder::PARAM_STR_ARRAY), IQueryBuilder::PARAM_STR) - ); - return $qb->executeStatement(); - } -} diff --git a/lib/Migration/Version035000Date20260529120000.php b/lib/Migration/Version035000Date20260529120000.php new file mode 100644 index 000000000..312b068f3 --- /dev/null +++ b/lib/Migration/Version035000Date20260529120000.php @@ -0,0 +1,198 @@ +hasTable('appconfig_ex')) { + $failed += $this->migrateAppConfig($output); + } + if ($schema->hasTable('preferences_ex')) { + $failed += $this->migratePreferences($output); + } + + // Persist the outcome so the table-drop migration can gate on it. Stored lazy under + // AppAPI's own app id; a value > 0 means some rows are still only in the legacy tables. + $this->appConfig->setValueString('app_api', self::FAILED_FLAG, (string)$failed, lazy: true); + if ($failed > 0) { + $output->warning(sprintf('Config/preferences backfill left %d row(s) un-migrated — investigate before dropping the legacy tables.', $failed)); + } + + return null; + } + + private function migrateAppConfig(IOutput $output): int { + $migrated = $skipped = $failed = 0; + + $lastId = 0; + while (($rows = $this->fetchBatch('appconfig_ex', ['appid', 'configkey', 'configvalue', 'sensitive'], $lastId)) !== []) { + foreach ($rows as $row) { + $lastId = (int)$row['id']; + $appId = (string)$row['appid']; + $configKey = (string)$row['configkey']; + $sensitive = (int)($row['sensitive'] ?? 0) === 1; + + try { + if ($this->appConfig->hasKey($appId, $configKey, null)) { + $skipped++; + continue; + } + $value = $this->decryptLegacyValue((string)($row['configvalue'] ?? ''), $sensitive); + if ($value === null) { + $output->warning(sprintf('Config migration: failed to decrypt sensitive value for app %s key %s (row id=%d) — skipping', $appId, $configKey, $lastId)); + $failed++; + continue; + } + $this->appConfig->setValueString($appId, $configKey, $value, lazy: true, sensitive: $sensitive); + $migrated++; + } catch (Throwable $e) { + $output->warning(sprintf('Config migration: failed to migrate app %s key %s (row id=%d): %s — skipping', $appId, $configKey, $lastId, $e->getMessage())); + $failed++; + } + } + } + + $output->info(sprintf('Config migration (appconfig_ex -> oc_appconfig): %d migrated, %d already present, %d failed', $migrated, $skipped, $failed)); + return $failed; + } + + private function migratePreferences(IOutput $output): int { + $migrated = $skipped = $failed = 0; + + $lastId = 0; + while (($rows = $this->fetchBatch('preferences_ex', ['userid', 'appid', 'configkey', 'configvalue', 'sensitive'], $lastId)) !== []) { + foreach ($rows as $row) { + $lastId = (int)$row['id']; + $userId = (string)$row['userid']; + $appId = (string)$row['appid']; + $configKey = (string)$row['configkey']; + $sensitive = (int)($row['sensitive'] ?? 0) === 1; + + try { + if ($this->userConfig->hasKey($userId, $appId, $configKey, null)) { + $skipped++; + continue; + } + $value = $this->decryptLegacyValue((string)($row['configvalue'] ?? ''), $sensitive); + if ($value === null) { + $output->warning(sprintf('Preferences migration: failed to decrypt sensitive value for user %s app %s key %s (row id=%d) — skipping', $userId, $appId, $configKey, $lastId)); + $failed++; + continue; + } + $this->userConfig->setValueString( + $userId, $appId, $configKey, $value, + lazy: true, + flags: $sensitive ? IUserConfig::FLAG_SENSITIVE : 0, + ); + $migrated++; + } catch (Throwable $e) { + $output->warning(sprintf('Preferences migration: failed to migrate user %s app %s key %s (row id=%d): %s — skipping', $userId, $appId, $configKey, $lastId, $e->getMessage())); + $failed++; + } + } + } + + $output->info(sprintf('Preferences migration (preferences_ex -> oc_preferences): %d migrated, %d already present, %d failed', $migrated, $skipped, $failed)); + return $failed; + } + + /** + * Decrypt a legacy value if it was stored encrypted. Mirrors the old services' guard: + * only `sensitive=1` rows with a non-empty value were ever encrypted (with the raw + * `ICrypto` envelope, no prefix). Returns the plaintext, or null if decryption failed. + */ + private function decryptLegacyValue(string $rawValue, bool $sensitive): ?string { + if (!$sensitive || $rawValue === '') { + return $rawValue; + } + try { + return $this->crypto->decrypt($rawValue); + } catch (Throwable) { + return null; + } + } + + /** + * Fetch up to {@see BATCH_SIZE} rows whose id is greater than `$afterId`, ordered by id, then + * close the cursor before the caller issues any writes. Keyset pagination keeps memory bounded on + * large tables and avoids holding a forward-only cursor open while writing config on the same + * connection. The backfill never modifies the legacy tables, so paging over them is stable. + * + * @param string[] $columns + * @return list> + */ + private function fetchBatch(string $table, array $columns, int $afterId): array { + $qb = $this->connection->getQueryBuilder(); + $qb->select('id', ...$columns) + ->from($table) + ->where($qb->expr()->gt('id', $qb->createNamedParameter($afterId, IQueryBuilder::PARAM_INT))) + ->orderBy('id', 'ASC') + ->setMaxResults(self::BATCH_SIZE); + $cursor = $qb->executeQuery(); + $rows = $cursor->fetchAll(); + $cursor->closeCursor(); + return $rows; + } +} diff --git a/lib/Migration/Version035000Date20260529130000.php b/lib/Migration/Version035000Date20260529130000.php new file mode 100644 index 000000000..f53cfc326 --- /dev/null +++ b/lib/Migration/Version035000Date20260529130000.php @@ -0,0 +1,126 @@ +backfillIsComplete($output)) { + return null; + } + + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + foreach (self::LEGACY_TABLES as $table) { + if ($schema->hasTable($table)) { + $schema->dropTable($table); + $this->dropped = true; + } + } + + return $this->dropped ? $schema : null; + } + + public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { + // The outcome flag only existed to gate this drop. Once no legacy table remains it is dead + // bookkeeping, so clear it whether this run dropped them or they were already gone. While a + // table still exists (e.g. the drop was skipped due to backfill failures) the flag is kept so + // a later re-run still sees the failure count. + if ($this->appConfig->hasKey('app_api', Version035000Date20260529120000::FAILED_FLAG, null) + && !$this->anyLegacyTableExists()) { + $this->appConfig->deleteKey('app_api', Version035000Date20260529120000::FAILED_FLAG); + } + return null; + } + + private function anyLegacyTableExists(): bool { + foreach (self::LEGACY_TABLES as $table) { + if ($this->connection->tableExists($table)) { + return true; + } + } + return false; + } + + /** + * Decide whether it is safe to drop the legacy tables. The backfill records its outcome in the + * app config key `app_api / `: + * - flag present and 0 -> backfill ran cleanly, safe to drop; + * - flag present and >0 -> some rows failed to migrate, keep the legacy tables as a recoverable copy; + * - flag absent -> the backfill has not recorded an outcome. That is normal on a fresh, + * schema-only install (where the legacy tables never hold data), but it + * also happens if this migration is run out of order / before the backfill. + * In that case we only drop when the legacy tables hold no data to lose. + */ + private function backfillIsComplete(IOutput $output): bool { + if ($this->appConfig->hasKey('app_api', Version035000Date20260529120000::FAILED_FLAG, null)) { + $failed = (int)$this->appConfig->getValueString('app_api', Version035000Date20260529120000::FAILED_FLAG, '0', lazy: true); + if ($failed > 0) { + $output->warning(sprintf( + 'Keeping appconfig_ex/preferences_ex: %d row(s) failed to backfill into oc_appconfig/oc_preferences. ' + . 'Resolve them, then re-run the upgrade to drop the legacy tables.', + $failed, + )); + return false; + } + return true; + } + + foreach (self::LEGACY_TABLES as $table) { + if ($this->connection->tableExists($table) && $this->tableHasRows($table)) { + $output->warning(sprintf( + 'Keeping %s: the config backfill (Version035000Date20260529120000) has not run yet. ' + . 'Run "occ upgrade" so the data is migrated before the legacy tables are dropped.', + $table, + )); + return false; + } + } + return true; + } + + private function tableHasRows(string $table): bool { + $qb = $this->connection->getQueryBuilder(); + $qb->select($qb->func()->count('*', 'cnt'))->from($table); + $result = $qb->executeQuery(); + $count = (int)$result->fetchOne(); + $result->closeCursor(); + return $count > 0; + } +} diff --git a/lib/Service/ExAppConfigService.php b/lib/Service/ExAppConfigService.php index 2999d185e..afe9989a4 100644 --- a/lib/Service/ExAppConfigService.php +++ b/lib/Service/ExAppConfigService.php @@ -10,93 +10,107 @@ namespace OCA\AppAPI\Service; use OCA\AppAPI\Db\ExAppConfig; -use OCA\AppAPI\Db\ExAppConfigMapper; -use OCP\AppFramework\Db\DoesNotExistException; -use OCP\AppFramework\Db\MultipleObjectsReturnedException; -use OCP\DB\Exception; -use OCP\Security\ICrypto; +use OCP\IAppConfig; +use OCP\IDBConnection; use Psr\Log\LoggerInterface; /** - * App configuration (appconfig_ex) + * App configuration backed by the server's standard `IAppConfig` storage (`oc_appconfig`). + * + * AppAPI historically kept its own `appconfig_ex` table; since the server gained typed, + * lazy and sensitive (encrypted) app config, that duplication is gone. All ExApp config + * values are stored as lazy strings; sensitive values are encrypted by the server. */ readonly class ExAppConfigService { public function __construct( - private ExAppConfigMapper $mapper, + private IAppConfig $appConfig, + private IDBConnection $connection, private LoggerInterface $logger, - private ICrypto $crypto, ) { } + /** + * Return the values of the requested keys that actually exist, in the shape + * `[['configkey' => ..., 'configvalue' => ...], ...]`. Sensitive values are + * returned decrypted (the server handles decryption transparently). + */ 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 = ''; - } + $values = []; + // array_unique: a single SQL `IN (...)` used to dedupe; preserve that so duplicate + // request keys don't yield duplicate result rows. + foreach (array_unique(array_map('strval', $configKeys)) as $configKey) { + // `null` lazy matches keys regardless of their lazy flag. + if (!$this->appConfig->hasKey($appId, $configKey, null)) { + continue; + } + try { + $configValue = $this->buildConfigValue($appId, $configKey)->getConfigvalue(); + } catch (\Throwable $e) { + $this->logger->warning(sprintf('Failed to read value for app %s, config key %s', $appId, $configKey), ['exception' => $e]); + $configValue = ''; } - return [ - 'configkey' => $exAppConfig->getConfigkey(), - 'configvalue' => $value, + $values[] = [ + 'configkey' => $configKey, + 'configvalue' => $configValue, ]; - }, $this->mapper->findByAppConfigKeys($appId, $configKeys)); - } catch (Exception) { - return null; + } + return $values; + } catch (\Throwable $e) { + $this->logger->warning(sprintf('Failed to get app config values for app %s', $appId), ['exception' => $e]); + return []; } } + /** + * Create or update an ExApp config value. + * + * When `$sensitive` is null the current sensitivity is preserved (or defaults to + * non-sensitive for a new key). The returned object always carries the plaintext value. + */ 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' => $sensitive ? $encryptedValue : $configValue ?? '', - 'sensitive' => $sensitive ?? 0, - ])); - } catch (Exception $e) { - $this->logger->error(sprintf('Failed to insert appconfig_ex value. Error: %s', $e->getMessage()), ['exception' => $e]); - return null; - } - } else { - $appConfigEx->setConfigvalue($sensitive ? $encryptedValue : $configValue); - if ($sensitive !== null) { - $appConfigEx->setSensitive($sensitive); - } - if ($this->updateAppConfigValue($appConfigEx) === null) { - $this->logger->error(sprintf('Error while updating appconfig_ex %s value.', $configKey)); - return null; + try { + $value = (string)($configValue ?? ''); + $currentSensitive = $this->appConfig->hasKey($appId, $configKey, null) + && $this->appConfig->isSensitive($appId, $configKey, null); + $targetSensitive = $sensitive !== null ? (bool)$sensitive : $currentSensitive; + + if ($currentSensitive && !$targetSensitive) { + $this->downgradeToPlain($appId, $configKey, $value); + } else { + $this->appConfig->setValueString($appId, $configKey, $value, lazy: true, sensitive: $targetSensitive); } + + return new ExAppConfig([ + 'appid' => $appId, + 'configkey' => $configKey, + 'configvalue' => $value, + 'sensitive' => $targetSensitive ? 1 : 0, + ]); + } catch (\Throwable $e) { + $this->logger->error(sprintf('Failed to set app config value for app %s, config key %s. Error: %s', $appId, $configKey, $e->getMessage()), ['exception' => $e]); + return null; } - if ($sensitive) { - // setting original unencrypted value for API - $appConfigEx->setConfigvalue($configValue); - } - return $appConfigEx; } + /** + * Delete the requested keys that exist; returns the number deleted, or -1 on error. + */ public function deleteAppConfigValues(array $configKeys, string $appId): int { try { - return $this->mapper->deleteByAppidConfigkeys($appId, $configKeys); - } catch (Exception) { + $deleted = 0; + foreach ($configKeys as $configKey) { + $configKey = (string)$configKey; + if ($this->appConfig->hasKey($appId, $configKey, null)) { + $this->appConfig->deleteKey($appId, $configKey); + $deleted++; + } + } + return $deleted; + } catch (\Throwable $e) { + $this->logger->error(sprintf('Failed to delete app config values for app %s. Error: %s', $appId, $e->getMessage()), ['exception' => $e]); return -1; } } @@ -106,33 +120,93 @@ public function deleteAppConfigValues(array $configKeys, string $appId): int { */ public function getAllAppConfig(string $appId): array { try { - return $this->mapper->findAllByAppid($appId); - } catch (Exception) { + $result = []; + foreach ($this->appConfig->getKeys($appId) as $configKey) { + try { + $result[] = $this->buildConfigValue($appId, $configKey); + } catch (\Throwable $e) { + // A single unreadable/type-conflicting key must not drop the whole listing. + $this->logger->warning(sprintf('Failed to read app config for app %s, config key %s — skipping', $appId, $configKey), ['exception' => $e]); + } + } + return $result; + } catch (\Throwable $e) { + $this->logger->warning(sprintf('Failed to list app config for app %s', $appId), ['exception' => $e]); return []; } } - public function getAppConfig(mixed $appId, mixed $configKey): ?ExAppConfig { + public function getAppConfig(string $appId, string $configKey): ?ExAppConfig { try { - return $this->mapper->findByAppConfigKey($appId, $configKey); - } catch (DoesNotExistException|MultipleObjectsReturnedException|Exception) { + if (!$this->appConfig->hasKey($appId, $configKey, null)) { + return null; + } + return $this->buildConfigValue($appId, $configKey); + } catch (\Throwable $e) { + $this->logger->warning(sprintf('Failed to get app config for app %s, config key %s', $appId, $configKey), ['exception' => $e]); return null; } } public function updateAppConfigValue(ExAppConfig $exAppConfig): ?ExAppConfig { + return $this->setAppConfigValue( + $exAppConfig->getAppid(), + $exAppConfig->getConfigkey(), + $exAppConfig->getConfigvalue(), + $exAppConfig->getSensitive(), + ); + } + + public function deleteAppConfig(ExAppConfig $exAppConfig): ?int { try { - return $this->mapper->update($exAppConfig); - } catch (Exception) { + $appId = $exAppConfig->getAppid(); + $configKey = $exAppConfig->getConfigkey(); + if (!$this->appConfig->hasKey($appId, $configKey, null)) { + return 0; + } + $this->appConfig->deleteKey($appId, $configKey); + return 1; + } catch (\Throwable $e) { + $this->logger->error(sprintf('Failed to delete app config for app %s, config key %s. Error: %s', $exAppConfig->getAppid(), $exAppConfig->getConfigkey(), $e->getMessage()), ['exception' => $e]); return null; } } - public function deleteAppConfig(ExAppConfig $exAppConfig): ?int { + /** + * Turn a currently-sensitive key into a plain value with the new content. + * + * IAppConfig won't unset sensitivity via setValueString() (it is sticky), and updateSensitive() + * refreshes the type cache but not the value cache. So we drop the key and re-create it as a + * plain value, which leaves both storage and the in-request cache correct. The two writes run + * in a transaction so a failure can never leave the key deleted (the legacy path was atomic). + */ + private function downgradeToPlain(string $appId, string $configKey, string $value): void { + $this->connection->beginTransaction(); try { - return $this->mapper->deleteByAppidConfigkeys($exAppConfig->getAppid(), [$exAppConfig->getConfigkey()]); - } catch (Exception) { - return null; + $this->appConfig->deleteKey($appId, $configKey); + $this->appConfig->setValueString($appId, $configKey, $value, lazy: true, sensitive: false); + $this->connection->commit(); + } catch (\Throwable $e) { + try { + $this->connection->rollBack(); + } catch (\Throwable) { + // rollBack on an already-aborted transaction is not actionable here. + } + throw $e; } } + + /** + * Build the value object for an existing key, reading it with its actual lazy flag + * so values created outside AppAPI (e.g. `occ config:app:set`) are handled too. + */ + private function buildConfigValue(string $appId, string $configKey): ExAppConfig { + $lazy = $this->appConfig->isLazy($appId, $configKey); + return new ExAppConfig([ + 'appid' => $appId, + 'configkey' => $configKey, + 'configvalue' => $this->appConfig->getValueString($appId, $configKey, '', lazy: $lazy), + 'sensitive' => $this->appConfig->isSensitive($appId, $configKey, $lazy) ? 1 : 0, + ]); + } } diff --git a/lib/Service/ExAppPreferenceService.php b/lib/Service/ExAppPreferenceService.php index 5dbfb2489..131b3c436 100644 --- a/lib/Service/ExAppPreferenceService.php +++ b/lib/Service/ExAppPreferenceService.php @@ -10,104 +10,137 @@ namespace OCA\AppAPI\Service; use OCA\AppAPI\Db\ExAppPreference; -use OCA\AppAPI\Db\ExAppPreferenceMapper; -use OCP\AppFramework\Db\DoesNotExistException; -use OCP\AppFramework\Db\MultipleObjectsReturnedException; -use OCP\DB\Exception; -use OCP\Security\ICrypto; +use OCP\Config\IUserConfig; +use OCP\IDBConnection; use Psr\Log\LoggerInterface; /** - * App per-user preferences (preferences_ex) + * App per-user preferences backed by the server's standard `IUserConfig` storage (`oc_preferences`). + * + * Replaces AppAPI's former `preferences_ex` table. Values are stored as lazy strings, scoped + * per user; sensitive values are encrypted by the server via the `FLAG_SENSITIVE` flag. */ readonly class ExAppPreferenceService { public function __construct( - private ExAppPreferenceMapper $mapper, + private IUserConfig $userConfig, + private IDBConnection $connection, private LoggerInterface $logger, - private ICrypto $crypto, ) { } + /** + * Create or update a per-user preference value. + * + * When `$sensitive` is null the current sensitivity is preserved (or defaults to + * non-sensitive for a new key). The returned object always carries the plaintext value. + */ 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 { - $exAppPreference = $this->mapper->insert(new ExAppPreference([ - 'userid' => $userId, - 'appid' => $appId, - 'configkey' => $configKey, - '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($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; - } - } catch (Exception $e) { - $this->logger->error('Error while updating config value: ' . $e->getMessage(), ['exception' => $e]); - return null; + $value = (string)($configValue ?? ''); + $currentSensitive = $this->userConfig->hasKey($userId, $appId, $configKey, null) + && $this->userConfig->isSensitive($userId, $appId, $configKey, null); + $targetSensitive = $sensitive !== null ? (bool)$sensitive : $currentSensitive; + + if ($currentSensitive && !$targetSensitive) { + $this->downgradeToPlain($userId, $appId, $configKey, $value); + } else { + $this->userConfig->setValueString( + $userId, $appId, $configKey, $value, + lazy: true, + flags: $targetSensitive ? IUserConfig::FLAG_SENSITIVE : 0, + ); } + + return new ExAppPreference([ + 'userid' => $userId, + 'appid' => $appId, + 'configkey' => $configKey, + 'configvalue' => $value, + 'sensitive' => $targetSensitive ? 1 : 0, + ]); + } catch (\Throwable $e) { + $this->logger->error(sprintf('Failed to set user config value for user %s, app %s, config key %s. Error: %s', $userId, $appId, $configKey, $e->getMessage()), ['exception' => $e]); + return null; } - if ($sensitive) { - // setting original unencrypted value for API - $exAppPreference->setConfigvalue($configValue); - } - return $exAppPreference; } + /** + * Return the values of the requested keys that actually exist, in the shape + * `[['configkey' => ..., 'configvalue' => ...], ...]`. Sensitive values are returned decrypted. + */ 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 = ''; - } + $values = []; + // array_unique: a single SQL `IN (...)` used to dedupe; preserve that so duplicate + // request keys don't yield duplicate result rows. + foreach (array_unique(array_map('strval', $configKeys)) as $configKey) { + // `null` lazy matches keys regardless of their lazy flag (e.g. eager values written + // through the server-native provisioning_api user-config endpoint). + if (!$this->userConfig->hasKey($userId, $appId, $configKey, null)) { + continue; + } + try { + $lazy = $this->userConfig->isLazy($userId, $appId, $configKey); + $configValue = $this->userConfig->getValueString($userId, $appId, $configKey, '', lazy: $lazy); + } catch (\Throwable $e) { + $this->logger->warning(sprintf('Failed to read value for user %s, app %s, config key %s', $userId, $appId, $configKey), ['exception' => $e]); + $configValue = ''; } - return [ - 'configkey' => $exAppPreference->getConfigkey(), - 'configvalue' => $value, + $values[] = [ + 'configkey' => $configKey, + 'configvalue' => $configValue, ]; - }, $this->mapper->findByUserIdAppIdKeys($userId, $appId, $configKeys)); - } catch (Exception) { - return null; + } + return $values; + } catch (\Throwable $e) { + $this->logger->warning(sprintf('Failed to get user config values for user %s, app %s', $userId, $appId), ['exception' => $e]); + return []; } } + /** + * Delete the requested keys that exist; returns the number deleted, or -1 on error. + */ public function deleteUserConfigValues(array $configKeys, string $userId, string $appId): int { try { - return $this->mapper->deleteUserConfigValues($configKeys, $userId, $appId); - } catch (Exception) { + $deleted = 0; + foreach ($configKeys as $configKey) { + $configKey = (string)$configKey; + if ($this->userConfig->hasKey($userId, $appId, $configKey, null)) { + $this->userConfig->deleteUserConfig($userId, $appId, $configKey); + $deleted++; + } + } + return $deleted; + } catch (\Throwable $e) { + $this->logger->error(sprintf('Failed to delete user config values for user %s, app %s. Error: %s', $userId, $appId, $e->getMessage()), ['exception' => $e]); return -1; } } + + /** + * Turn a currently-sensitive key into a plain value with the new content. + * + * IUserConfig won't unset sensitivity via setValueString() (it is sticky), and updateSensitive() + * refreshes the type cache but not the value cache. So we drop the key and re-create it as a + * plain value, which leaves both storage and the in-request cache correct. The two writes run + * in a transaction so a failure can never leave the key deleted (the legacy path was atomic). + */ + private function downgradeToPlain(string $userId, string $appId, string $configKey, string $value): void { + $this->connection->beginTransaction(); + try { + $this->userConfig->deleteUserConfig($userId, $appId, $configKey); + $this->userConfig->setValueString($userId, $appId, $configKey, $value, lazy: true, flags: 0); + $this->connection->commit(); + } catch (\Throwable $e) { + try { + $this->connection->rollBack(); + } catch (\Throwable) { + // rollBack on an already-aborted transaction is not actionable here. + } + throw $e; + } + } } diff --git a/tests/php/Controller/AppConfigControllerTest.php b/tests/php/Controller/AppConfigControllerTest.php index 6158cd863..dbc20d4c4 100644 --- a/tests/php/Controller/AppConfigControllerTest.php +++ b/tests/php/Controller/AppConfigControllerTest.php @@ -14,6 +14,7 @@ use OCP\AppFramework\Http; use OCP\AppFramework\OCS\OCSBadRequestException; use OCP\AppFramework\OCS\OCSNotFoundException; +use OCP\IAppConfig; use OCP\IRequest; use OCP\Server; use PHPUnit\Framework\Attributes\Group; @@ -104,4 +105,40 @@ public function testDeleteMissingThrowsNotFound(): void { $this->expectException(OCSNotFoundException::class); $this->controller->deleteAppConfigValues(['no_such_key_phpunit_xyz']); } + + public function testSensitiveDowngradeClearsFlag(): void { + // IAppConfig keeps sensitivity sticky; the service must force the downgrade so an + // explicit sensitive=0 on a previously-sensitive key actually unsets it (and the value + // stays readable as plaintext). This mirrors nc_py_api's appcfg_prefs_ex sensitive test. + $this->controller->setAppConfigValue(self::KEY_SECRET, '123', sensitive: 1); + $response = $this->controller->setAppConfigValue(self::KEY_SECRET, '123', sensitive: 0); + self::assertSame(0, $response->getData()->getSensitive()); + + $rows = $this->controller->getAppConfigValues([self::KEY_SECRET])->getData(); + self::assertSame('123', array_column($rows, 'configvalue', 'configkey')[self::KEY_SECRET]); + } + + public function testEmptySensitiveValueRoundTrips(): void { + // Empty + sensitive: server encrypts an empty string; read must return '' (not the envelope). + $this->controller->setAppConfigValue(self::KEY_SECRET, '', sensitive: 1); + $rows = $this->controller->getAppConfigValues([self::KEY_SECRET])->getData(); + self::assertSame('', array_column($rows, 'configvalue', 'configkey')[self::KEY_SECRET]); + } + + public function testNonLazyKeyIsReadableAndDeletable(): void { + // A key created non-lazy (e.g. via `occ config:app:set` or a colliding PHP app) must still be + // readable and deletable through the service — existence checks use lazy-agnostic hasKey(null). + $appConfig = Server::get(IAppConfig::class); + $appConfig->setValueString(self::TEST_APP_ID, self::KEY_PLAIN, 'nonlazyval', lazy: false); + try { + $rows = $this->controller->getAppConfigValues([self::KEY_PLAIN])->getData(); + self::assertSame('nonlazyval', array_column($rows, 'configvalue', 'configkey')[self::KEY_PLAIN] ?? null); + $del = $this->controller->deleteAppConfigValues([self::KEY_PLAIN]); + self::assertSame(Http::STATUS_OK, $del->getStatus(), 'non-lazy key must be deletable'); + } finally { + if ($appConfig->hasKey(self::TEST_APP_ID, self::KEY_PLAIN, null)) { + $appConfig->deleteKey(self::TEST_APP_ID, self::KEY_PLAIN); + } + } + } } diff --git a/tests/php/Controller/PreferencesControllerTest.php b/tests/php/Controller/PreferencesControllerTest.php new file mode 100644 index 000000000..1426ad470 --- /dev/null +++ b/tests/php/Controller/PreferencesControllerTest.php @@ -0,0 +1,118 @@ +request = $this->createMock(IRequest::class); + $this->request->method('getHeader')->willReturnCallback( + fn (string $name): string => $name === 'EX-APP-ID' ? self::TEST_APP_ID : '' + ); + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn(self::TEST_USER_ID); + $userSession = $this->createMock(IUserSession::class); + $userSession->method('getUser')->willReturn($user); + + $this->service = Server::get(ExAppPreferenceService::class); + $this->controller = new PreferencesController($this->request, $userSession, $this->service); + $this->cleanup(); + } + + protected function tearDown(): void { + $this->cleanup(); + parent::tearDown(); + } + + private function cleanup(): void { + $this->service->deleteUserConfigValues([self::KEY_PLAIN, self::KEY_SECRET], self::TEST_USER_ID, self::TEST_APP_ID); + } + + public function testSensitiveFlagPersistsOnSet(): void { + $response = $this->controller->setUserConfigValue(self::KEY_SECRET, '123', sensitive: 1); + self::assertSame(Http::STATUS_OK, $response->getStatus()); + self::assertSame('123', $response->getData()->getConfigvalue()); + self::assertSame(1, $response->getData()->getSensitive()); + } + + public function testNonSensitiveDefaultsToZero(): void { + $response = $this->controller->setUserConfigValue(self::KEY_PLAIN, 'plain', sensitive: null); + self::assertSame(0, $response->getData()->getSensitive()); + } + + public function testSensitiveFlagPreservedOnUpdateWithoutFlag(): void { + $this->controller->setUserConfigValue(self::KEY_SECRET, 'orig', sensitive: 1); + $response = $this->controller->setUserConfigValue(self::KEY_SECRET, 'updated', sensitive: null); + self::assertSame(1, $response->getData()->getSensitive()); + } + + public function testSensitiveDowngradeClearsFlag(): void { + $this->controller->setUserConfigValue(self::KEY_SECRET, '123', sensitive: 1); + $response = $this->controller->setUserConfigValue(self::KEY_SECRET, '123', sensitive: 0); + self::assertSame(0, $response->getData()->getSensitive()); + $rows = $this->controller->getUserConfigValues([self::KEY_SECRET])->getData(); + self::assertSame('123', array_column($rows, 'configvalue', 'configkey')[self::KEY_SECRET]); + } + + public function testGetReturnsDecryptedValues(): void { + $this->controller->setUserConfigValue(self::KEY_PLAIN, 'a', sensitive: 0); + $this->controller->setUserConfigValue(self::KEY_SECRET, 'b', sensitive: 1); + + $rows = $this->controller->getUserConfigValues([self::KEY_PLAIN, self::KEY_SECRET])->getData(); + self::assertCount(2, $rows); + $byKey = array_column($rows, 'configvalue', 'configkey'); + self::assertSame('a', $byKey[self::KEY_PLAIN]); + self::assertSame('b', $byKey[self::KEY_SECRET]); + } + + public function testEmptySensitiveValueRoundTrips(): void { + $this->controller->setUserConfigValue(self::KEY_SECRET, '', sensitive: 1); + $rows = $this->controller->getUserConfigValues([self::KEY_SECRET])->getData(); + self::assertSame('', array_column($rows, 'configvalue', 'configkey')[self::KEY_SECRET]); + } + + public function testEmptyKeyRejected(): void { + $this->expectException(OCSBadRequestException::class); + $this->controller->setUserConfigValue('', 'x'); + } + + public function testDeleteMissingThrowsNotFound(): void { + $this->expectException(OCSNotFoundException::class); + $this->controller->deleteUserConfigValues(['no_such_key_phpunit_xyz']); + } +} diff --git a/tests/php/Migration/Version034000Date20260428144801Test.php b/tests/php/Migration/Version034000Date20260428144801Test.php deleted file mode 100644 index 8661b8fce..000000000 --- a/tests/php/Migration/Version034000Date20260428144801Test.php +++ /dev/null @@ -1,229 +0,0 @@ -db = Server::get(IDBConnection::class); - $this->crypto = Server::get(ICrypto::class); - $this->migration = new Version034000Date20260428144801( - $this->db, - $this->crypto, - ); - $this->cleanupAll(); - } - - protected function tearDown(): void { - $this->cleanupAll(); - parent::tearDown(); - } - - public function testPlaintextSecretMigratesAndRoundTrips(): void { - // Legacy reality: TalkBotsService::setAppConfigValue() was called without $sensitive=true, - // so bot secrets sit in appconfig_ex as plaintext. The migration must accept this and - // re-encrypt on insert into the new table. - $plaintext = str_repeat('A', 64); - $this->seedLegacyBot(self::TEST_APP_ONE, 'plain_route', $plaintext, sensitive: 0); - - $this->migration->postSchemaChange(new \OC\Migration\NullOutput(), fn () => $this->schema(), []); - - $row = $this->fetchExAppsTalkBotsRow(self::TEST_APP_ONE, 'plain_route'); - self::assertNotNull($row, 'bot row must exist post-migration'); - self::assertSame($plaintext, $this->crypto->decrypt($row['secret'])); - self::assertSame(0, $this->countLegacyRowsFor(self::TEST_APP_ONE), 'legacy rows must be purged'); - } - - public function testSensitiveEncryptedSecretDecryptsThenReEncrypts(): void { - // Hypothetical scenario: an operator manually flipped sensitive=1 and let Version032002 - // re-encrypt the row in place. The migration's `if (sensitive == 1)` branch must decrypt - // with the same key and re-encrypt for the new table. - $plaintext = str_repeat('B', 64); - $encryptedLegacy = $this->crypto->encrypt($plaintext); - $this->seedLegacyBot(self::TEST_APP_ONE, 'sensitive_route', $encryptedLegacy, sensitive: 1); - - $this->migration->postSchemaChange(new \OC\Migration\NullOutput(), fn () => $this->schema(), []); - - $row = $this->fetchExAppsTalkBotsRow(self::TEST_APP_ONE, 'sensitive_route'); - self::assertNotNull($row); - self::assertSame($plaintext, $this->crypto->decrypt($row['secret'])); - } - - public function testOrphanRouteRowSkippedAndPreserved(): void { - // A route row without its matching secret row: defensive skip, no insert, no delete — - // re-running the migration after the operator fixes the data must still succeed. - $route = 'orphan_route'; - $hash = sha1(self::TEST_APP_ONE . '_' . $route); - $this->insertAppConfigEx(self::TEST_APP_ONE, self::TALK_BOT_ROUTE_PREFIX . $hash, $route, 0); - - $this->migration->postSchemaChange(new \OC\Migration\NullOutput(), fn () => $this->schema(), []); - - self::assertNull($this->fetchExAppsTalkBotsRow(self::TEST_APP_ONE, $route)); - self::assertSame(1, $this->countLegacyRowsFor(self::TEST_APP_ONE), 'orphan route must NOT be deleted'); - } - - public function testMalformedKeySuffixSkipped(): void { - // A talk_bot_route_* row whose suffix does not match sha1(appid_route) is either corrupt - // or planted by an external writer. Don't trust the configvalue; skip and preserve. - $route = 'mismatched_route'; - $this->insertAppConfigEx(self::TEST_APP_ONE, self::TALK_BOT_ROUTE_PREFIX . str_repeat('0', 40), $route, 0); - // Add a (different-key) secret row so we can prove the migration didn't consume it either. - $this->insertAppConfigEx(self::TEST_APP_ONE, str_repeat('0', 40), str_repeat('X', 64), 0); - - $this->migration->postSchemaChange(new \OC\Migration\NullOutput(), fn () => $this->schema(), []); - - self::assertNull($this->fetchExAppsTalkBotsRow(self::TEST_APP_ONE, $route)); - self::assertSame(2, $this->countLegacyRowsFor(self::TEST_APP_ONE), 'malformed pair must be preserved as-is'); - } - - public function testMultiBotMultiApp(): void { - // Three bots across two ExApps in a single migration pass. The unique (appid, route) - // constraint means each pair lands in its own row, even when routes collide across apps. - $this->seedLegacyBot(self::TEST_APP_ONE, 'shared_route', str_repeat('A', 64), 0); - $this->seedLegacyBot(self::TEST_APP_ONE, 'second_route', str_repeat('B', 64), 0); - $this->seedLegacyBot(self::TEST_APP_TWO, 'shared_route', str_repeat('C', 64), 0); - - $this->migration->postSchemaChange(new \OC\Migration\NullOutput(), fn () => $this->schema(), []); - - self::assertNotNull($this->fetchExAppsTalkBotsRow(self::TEST_APP_ONE, 'shared_route')); - self::assertNotNull($this->fetchExAppsTalkBotsRow(self::TEST_APP_ONE, 'second_route')); - self::assertNotNull($this->fetchExAppsTalkBotsRow(self::TEST_APP_TWO, 'shared_route')); - self::assertSame(0, $this->countLegacyRowsFor(self::TEST_APP_ONE)); - self::assertSame(0, $this->countLegacyRowsFor(self::TEST_APP_TWO)); - } - - public function testIdempotentRerun(): void { - // `occ migrations:execute` is the operator's escape hatch when a previous run was - // interrupted. Running the migration twice on the same data must not duplicate rows - // or fail; the second pass should see 0 candidate route rows. - $this->seedLegacyBot(self::TEST_APP_ONE, 'idempotent_route', str_repeat('D', 64), 0); - $this->migration->postSchemaChange(new \OC\Migration\NullOutput(), fn () => $this->schema(), []); - $first = $this->fetchExAppsTalkBotsRow(self::TEST_APP_ONE, 'idempotent_route'); - - $this->migration->postSchemaChange(new \OC\Migration\NullOutput(), fn () => $this->schema(), []); - $second = $this->fetchExAppsTalkBotsRow(self::TEST_APP_ONE, 'idempotent_route'); - - self::assertNotNull($first); - self::assertNotNull($second); - self::assertSame((int)$first['id'], (int)$second['id'], 'idempotent re-run must NOT mint a new row'); - } - - public function testRerunAfterPreviousMigratedRow(): void { - // Simulates: operator re-runs after fixing the data — the previously-migrated bot is - // still in ex_apps_talk_bots, AND a fresh legacy pair shows up for the SAME route. - // The migration must skip the insert (UNIQUE constraint would fire), still clean the - // stale legacy pair, and not corrupt the existing row. - $this->seedLegacyBot(self::TEST_APP_ONE, 'preexist_route', str_repeat('E', 64), 0); - $this->migration->postSchemaChange(new \OC\Migration\NullOutput(), fn () => $this->schema(), []); - $beforeId = (int)$this->fetchExAppsTalkBotsRow(self::TEST_APP_ONE, 'preexist_route')['id']; - - // Re-seed the legacy pair (e.g. someone restored from a partial backup). - $this->seedLegacyBot(self::TEST_APP_ONE, 'preexist_route', str_repeat('F', 64), 0); - $this->migration->postSchemaChange(new \OC\Migration\NullOutput(), fn () => $this->schema(), []); - - $row = $this->fetchExAppsTalkBotsRow(self::TEST_APP_ONE, 'preexist_route'); - self::assertSame($beforeId, (int)$row['id'], 'pre-existing row must be untouched'); - self::assertSame(0, $this->countLegacyRowsFor(self::TEST_APP_ONE), 'stale legacy pair must still be cleaned'); - } - - private function seedLegacyBot(string $appId, string $route, string $secretValue, int $sensitive): void { - $hash = sha1($appId . '_' . $route); - $this->insertAppConfigEx($appId, $hash, $secretValue, $sensitive); - $this->insertAppConfigEx($appId, self::TALK_BOT_ROUTE_PREFIX . $hash, $route, 0); - } - - private function insertAppConfigEx(string $appId, string $configKey, string $configValue, int $sensitive): void { - $qb = $this->db->getQueryBuilder(); - $qb->insert('appconfig_ex') - ->values([ - 'appid' => $qb->createNamedParameter($appId), - 'configkey' => $qb->createNamedParameter($configKey), - 'configvalue' => $qb->createNamedParameter($configValue), - 'sensitive' => $qb->createNamedParameter($sensitive, \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_INT), - ]); - $qb->executeStatement(); - } - - private function fetchExAppsTalkBotsRow(string $appId, string $route): ?array { - $qb = $this->db->getQueryBuilder(); - $qb->select('*') - ->from('ex_apps_talk_bots') - ->where( - $qb->expr()->eq('appid', $qb->createNamedParameter($appId)), - $qb->expr()->eq('route', $qb->createNamedParameter($route)), - ) - ->setMaxResults(1); - $res = $qb->executeQuery(); - $row = $res->fetch(); - $res->closeCursor(); - return $row === false ? null : $row; - } - - private function countLegacyRowsFor(string $appId): int { - $qb = $this->db->getQueryBuilder(); - $qb->select($qb->func()->count('*', 'cnt')) - ->from('appconfig_ex') - ->where($qb->expr()->eq('appid', $qb->createNamedParameter($appId))); - $res = $qb->executeQuery(); - $row = $res->fetch(); - $res->closeCursor(); - return (int)$row['cnt']; - } - - private function cleanupAll(): void { - foreach ([self::TEST_APP_ONE, self::TEST_APP_TWO] as $appId) { - $qb = $this->db->getQueryBuilder(); - $qb->delete('appconfig_ex') - ->where($qb->expr()->eq('appid', $qb->createNamedParameter($appId))); - $qb->executeStatement(); - - $qb = $this->db->getQueryBuilder(); - $qb->delete('ex_apps_talk_bots') - ->where($qb->expr()->eq('appid', $qb->createNamedParameter($appId))); - $qb->executeStatement(); - } - } - - private function schema(): ISchemaWrapper { - // Server::get(IDBConnection::class) returns the public ConnectionAdapter, but SchemaWrapper - // needs the internal OC\DB\Connection it wraps. Reflect once at test setup time — the - // `inner` field on ConnectionAdapter is declared private but stable across NC versions. - $inner = (new \ReflectionProperty(\OC\DB\ConnectionAdapter::class, 'inner'))->getValue($this->db); - return new \OC\DB\SchemaWrapper($inner); - } -} diff --git a/tests/php/Service/StorageLazyLoadingTest.php b/tests/php/Service/StorageLazyLoadingTest.php new file mode 100644 index 000000000..3564d258b --- /dev/null +++ b/tests/php/Service/StorageLazyLoadingTest.php @@ -0,0 +1,129 @@ +configService = Server::get(ExAppConfigService::class); + $this->preferenceService = Server::get(ExAppPreferenceService::class); + $this->appConfig = Server::get(IAppConfig::class); + $this->userConfig = Server::get(IUserConfig::class); + $this->db = Server::get(IDBConnection::class); + $this->cleanup(); + } + + protected function tearDown(): void { + $this->cleanup(); + parent::tearDown(); + } + + public function testConfigValuesAreStoredLazy(): void { + $this->configService->setAppConfigValue(self::APP, 'k', 'v'); + self::assertTrue($this->appConfig->isLazy(self::APP, 'k'), 'AppAPI config must be stored lazy'); + } + + public function testLazyConfigIsNotInEagerCacheButReadableViaLazyLoad(): void { + $this->configService->setAppConfigValue(self::APP, 'k', 'v'); + $this->appConfig->clearCache(); + // A lazy value must be absent from the eager (non-lazy) cache that loads on every request... + self::assertSame('', $this->appConfig->getValueString(self::APP, 'k', '', lazy: false)); + // ...but present once the lazy bucket is loaded. + self::assertSame('v', $this->appConfig->getValueString(self::APP, 'k', '', lazy: true)); + } + + public function testLazyConfigReadableFromColdCacheViaService(): void { + $this->configService->setAppConfigValue(self::APP, 'k', 'v'); + $this->appConfig->clearCache(); // simulate a fresh request with nothing pre-loaded + $rows = $this->configService->getAppConfigValues(self::APP, ['k']); + self::assertSame('v', $this->valueOf($rows, 'k')); + } + + public function testSensitiveLazyConfigRoundTripsFromColdCache(): void { + $this->configService->setAppConfigValue(self::APP, 'secret', 's3cr3t', sensitive: 1); + $this->appConfig->clearCache(); + self::assertTrue($this->appConfig->isLazy(self::APP, 'secret')); + self::assertTrue($this->appConfig->isSensitive(self::APP, 'secret', null)); + $rows = $this->configService->getAppConfigValues(self::APP, ['secret']); + self::assertSame('s3cr3t', $this->valueOf($rows, 'secret')); + } + + public function testEagerExternalConfigIsStillReadable(): void { + // Simulate a value created outside AppAPI (e.g. `occ config:app:set` without --lazy): eager. + $this->appConfig->setValueString(self::APP, 'eager_key', 'eagerval', lazy: false); + $this->appConfig->clearCache(); + $rows = $this->configService->getAppConfigValues(self::APP, ['eager_key']); + self::assertSame('eagerval', $this->valueOf($rows, 'eager_key'), 'eager keys must not be invisible to the service'); + } + + public function testPreferenceValuesAreStoredLazy(): void { + $this->preferenceService->setUserConfigValue(self::USER, self::APP, 'k', 'v'); + self::assertTrue($this->userConfig->isLazy(self::USER, self::APP, 'k'), 'AppAPI preferences must be stored lazy'); + } + + public function testEagerExternalPreferenceIsStillReadable(): void { + // The server-native provisioning_api user-config endpoint writes preferences EAGER. + // Our read path must surface them too, or an ExApp mixing both APIs would lose data. + $this->userConfig->setValueString(self::USER, self::APP, 'srv_pref', 'srvval', lazy: false); + $this->userConfig->clearCache(self::USER); + $rows = $this->preferenceService->getUserConfigValues(self::USER, self::APP, ['srv_pref']); + self::assertSame('srvval', $this->valueOf($rows, 'srv_pref'), 'eager (server-native) prefs must be readable'); + } + + private function valueOf(?array $rows, string $key): ?string { + foreach ($rows ?? [] as $row) { + if ($row['configkey'] === $key) { + return $row['configvalue']; + } + } + return null; + } + + private function cleanup(): void { + // This test only writes through the services / IAppConfig / IUserConfig, i.e. into the + // standard oc_appconfig / oc_preferences tables. It must NOT touch the legacy appconfig_ex / + // preferences_ex tables, which the drop migration removes (they're absent in a fresh CI DB). + foreach (['appconfig', 'preferences'] as $table) { + $qb = $this->db->getQueryBuilder(); + $qb->delete($table)->where($qb->expr()->eq('appid', $qb->createNamedParameter(self::APP))); + $qb->executeStatement(); + } + $this->appConfig->clearCache(); + $this->userConfig->clearCacheAll(); + } +}