fix(security): read the API key via IAppConfig, not IConfig

Regression in the sensitive-api-key change: EtherpadClient::getApiKey()
read the key through IConfig::getAppValue, which does NOT decrypt a
value stored sensitive (encrypted at rest). So every real Etherpad call
(createGroup, createAuthorIfNotExistsFor, createSession, …) sent an
unusable key and got HTTP 401 — opening any pad failed with
"Etherpad API request failed".

The admin health-check kept passing because it reads the key via
AdminSettingsRepository::getStoredSettings() → IAppConfig (which
decrypts), masking the bug; only the EtherpadClient path was broken.

Inject IAppConfig into EtherpadClient and read the key via
getValueString. Host/api-version stay on IConfig (not sensitive).

Verified on a real NC 33 install with the key stored sensitive: the
pad-author-display-name and pad-create-public e2e specs (which open
real pads through EtherpadClient) now pass; they failed with the 401
before this fix.

Author: Jaggob

lib/Service/EtherpadClient.php

@@ -9,6 +9,7 @@
 namespace OCA\EtherpadNextcloud\Service;
 
 use OCA\EtherpadNextcloud\Exception\EtherpadClientException;
+use OCP\IAppConfig;
 use OCP\IConfig;
 
 class EtherpadClient {
@@ -18,6 +19,7 @@ class EtherpadClient {
 
 	public function __construct(
 		private IConfig $config,
+		private IAppConfig $appConfig,
 	) {
 	}
 
@@ -297,10 +299,12 @@ private function getApiHost(): string {
 	}
 
 	private function getApiKey(): string {
-		// Read via IConfig (which transparently decrypts the now-sensitive
-		// value); the key name is owned by AdminSettingsRepository so the
-		// write and read paths can't drift apart.
-		$key = (string)$this->config->getAppValue('etherpad_nextcloud', AdminSettingsRepository::API_KEY, '');
+		// Must read via IAppConfig: the key is stored sensitive (encrypted
+		// at rest) and IConfig::getAppValue does NOT decrypt sensitive
+		// values, so reading it through IConfig yields an unusable key and
+		// every Etherpad call 401s. IAppConfig::getValueString decrypts it.
+		// The key name is owned by AdminSettingsRepository.
+		$key = $this->appConfig->getValueString('etherpad_nextcloud', AdminSettingsRepository::API_KEY, '');
 		if ($key === '') {
 			throw new EtherpadClientException('Etherpad API key is not configured.');
 		}

tests/phpunit/unit/EtherpadClientTest.php

@@ -5,6 +5,7 @@
 namespace OCA\EtherpadNextcloud\Tests\Unit;
 
 use OCA\EtherpadNextcloud\Service\EtherpadClient;
+use OCP\IAppConfig;
 use OCP\IConfig;
 use PHPUnit\Framework\TestCase;
 
@@ -20,44 +21,53 @@ static function (string $appName, string $key, string $default = ''): string {
 			}
 		);
 
-		$client = new EtherpadClient($config);
+		$client = $this->client($config);
 		$this->assertSame(
 			'https://pad.example.test/p/g.group%24pad-name',
 			$client->buildPadUrl('g.group$pad-name')
 		);
 	}
 
 	public function testGetConfiguredOriginNormalizesScheme(): void {
-		$client = new EtherpadClient($this->configWithHost('HTTPS://Pad.Example.Test/'));
+		$client = $this->client($this->configWithHost('HTTPS://Pad.Example.Test/'));
 		$this->assertSame('https://pad.example.test', $client->getConfiguredOrigin());
 	}
 
 	public function testGetConfiguredOriginOmitsDefaultPorts(): void {
-		$client = new EtherpadClient($this->configWithHost('https://pad.example.test:443'));
+		$client = $this->client($this->configWithHost('https://pad.example.test:443'));
 		$this->assertSame('https://pad.example.test', $client->getConfiguredOrigin());
 
-		$client = new EtherpadClient($this->configWithHost('http://pad.example.test:80'));
+		$client = $this->client($this->configWithHost('http://pad.example.test:80'));
 		$this->assertSame('http://pad.example.test', $client->getConfiguredOrigin());
 	}
 
 	public function testGetConfiguredOriginKeepsNonDefaultPort(): void {
-		$client = new EtherpadClient($this->configWithHost('https://pad.example.test:9001'));
+		$client = $this->client($this->configWithHost('https://pad.example.test:9001'));
 		$this->assertSame('https://pad.example.test:9001', $client->getConfiguredOrigin());
 	}
 
 	public function testGetConfiguredOriginAllowsHttp(): void {
 		// Unlike `parsePublicPadUrl`, the configured-origin accessor must not
 		// enforce https — admins may legitimately run Etherpad on http behind
 		// a private network.
-		$client = new EtherpadClient($this->configWithHost('http://pad.internal.lan'));
+		$client = $this->client($this->configWithHost('http://pad.internal.lan'));
 		$this->assertSame('http://pad.internal.lan', $client->getConfiguredOrigin());
 	}
 
 	public function testGetConfiguredOriginReturnsEmptyWhenUnconfigured(): void {
-		$client = new EtherpadClient($this->configWithHost(''));
+		$client = $this->client($this->configWithHost(''));
 		$this->assertSame('', $client->getConfiguredOrigin());
 	}
 
+	/**
+	 * Construct the client with a bare IAppConfig mock. None of these tests
+	 * exercise the API-key read path (no network calls), so the default
+	 * empty getValueString return is fine.
+	 */
+	private function client(IConfig $config): EtherpadClient {
+		return new EtherpadClient($config, $this->createMock(IAppConfig::class));
+	}
+
 	private function configWithHost(string $host): IConfig {
 		$config = $this->createMock(IConfig::class);
 		$config->method('getAppValue')->willReturnCallback(