fixup! fixup! feat(appconfig): Automatically store "sensitive" appconfigs encrypted in the database

Author: nickvergessen

lib/private/AppConfig.php

@@ -71,6 +71,8 @@
 class AppConfig implements IAppConfig {
 	private const APP_MAX_LENGTH = 32;
 	private const KEY_MAX_LENGTH = 64;
+	private const ENCRYPTION_PREFIX = '$AppConfigEncryption$';
+	private const ENCRYPTION_PREFIX_LENGTH = 21; // strlen(self::ENCRYPTION_PREFIX)
 
 	/** @var array<string, array<string, mixed>> ['app_id' => ['config_key' => 'config_value']] */
 	private array $fastCache = [];   // cache for normal config keys
@@ -473,12 +475,9 @@ private function getTypedValue(
 		}
 
 		$sensitive = $this->isTyped(self::VALUE_SENSITIVE, $knownType);
-		if ($sensitive) {
+		if ($sensitive && str_starts_with($value, self::ENCRYPTION_PREFIX)) {
 			// Only decrypt values that are stored encrypted
-			$sections = substr_count($value, '|');
-			if ($sections === 2 || $sections === 3) {
-				$value = $this->crypto->decrypt($value);
-			}
+			$value = $this->crypto->decrypt(substr($value, self::ENCRYPTION_PREFIX_LENGTH));
 		}
 
 		return $value;
@@ -751,7 +750,7 @@ private function setTypedValue(
 		}
 
 		if ($sensitive) {
-			$value = $this->crypto->encrypt($value);
+			$value = self::ENCRYPTION_PREFIX . $this->crypto->encrypt($value);
 		}
 
 		$refreshCache = false;

tests/lib/AppConfigTest.php

@@ -250,17 +250,7 @@ public function testDeleteKey(): void {
 
 		$this->assertFalse($config->hasKey('testapp', 'deletethis'));
 
-		$sql = $this->connection->getQueryBuilder();
-		$sql->select('configvalue')
-			->from('appconfig')
-			->where($sql->expr()->eq('appid', $sql->createParameter('appid')))
-			->andWhere($sql->expr()->eq('configkey', $sql->createParameter('configkey')))
-			->setParameter('appid', 'testapp')
-			->setParameter('configkey', 'deletethis');
-		$query = $sql->executeQuery();
-		$result = $query->fetch();
-		$query->closeCursor();
-		$this->assertFalse($result);
+		$this->assertFalse($this->loadConfigValueFromDatabase('testapp', 'deletethis'));
 	}
 
 	public function testDeleteApp(): void {
@@ -396,7 +386,7 @@ public function testMigratingNonSensitiveValueToSensitiveOne(): void {
 		$this->assertEquals($secret, $actualSecret);
 	}
 
-	protected function assertConfigKey(string $app, string $key, string $expected): void {
+	protected function loadConfigValueFromDatabase(string $app, string $key): string|false {
 		$sql = $this->connection->getQueryBuilder();
 		$sql->select('configvalue')
 			->from('appconfig')
@@ -405,24 +395,17 @@ protected function assertConfigKey(string $app, string $key, string $expected):
 			->setParameter('appid', $app)
 			->setParameter('configkey', $key);
 		$query = $sql->executeQuery();
-		$actual = $query->fetch();
+		$actual = $query->fetchOne();
 		$query->closeCursor();
 
-		$this->assertEquals($expected, $actual['configvalue']);
+		return $actual;
 	}
 
-	protected function assertConfigValueNotEquals(string $app, string $key, string $expected): void {
-		$sql = $this->connection->getQueryBuilder();
-		$sql->select('configvalue')
-			->from('appconfig')
-			->where($sql->expr()->eq('appid', $sql->createParameter('appid')))
-			->andWhere($sql->expr()->eq('configkey', $sql->createParameter('configkey')))
-			->setParameter('appid', $app)
-			->setParameter('configkey', $key);
-		$query = $sql->executeQuery();
-		$actual = $query->fetch();
-		$query->closeCursor();
+	protected function assertConfigKey(string $app, string $key, string|false $expected): void {
+		$this->assertEquals($expected, $this->loadConfigValueFromDatabase($app, $key));
+	}
 
-		$this->assertNotEquals($expected, $actual['configvalue']);
+	protected function assertConfigValueNotEquals(string $app, string $key, string|false $expected): void {
+		$this->assertNotEquals($expected, $this->loadConfigValueFromDatabase($app, $key));
 	}
 }