diff --git a/lib/Settings/Admin.php b/lib/Settings/Admin.php index a0c8caa..5602b7e 100644 --- a/lib/Settings/Admin.php +++ b/lib/Settings/Admin.php @@ -13,6 +13,7 @@ use OCP\IConfig; use OCP\Security\ICrypto; use OCP\Settings\ISettings; +use Psr\Log\LoggerInterface; class Admin implements ISettings { @@ -20,6 +21,7 @@ public function __construct( private IConfig $config, private ICrypto $crypto, private IInitialState $initialStateService, + private LoggerInterface $logger, ) { } @@ -28,7 +30,20 @@ public function __construct( */ public function getForm(): TemplateResponse { $apiKey = $this->config->getAppValue(Application::APP_ID, 'api_key'); - $apiKey = $apiKey === '' ? '' : $this->crypto->decrypt($apiKey); + if ($apiKey !== '') { + try { + $apiKey = $this->crypto->decrypt($apiKey); + } catch (\Exception $e) { + // Every app's admin section is rendered by the same controller, so + // throwing here does not just break ours: it makes the whole + // "Connected accounts" page fail. Treat an undecryptable key as unset. + $this->logger->warning('Could not decrypt the Giphy API key, treating it as unset', [ + 'exception' => $e, + 'app' => Application::APP_ID, + ]); + $apiKey = ''; + } + } $linkPreviewEnabled = $this->config->getAppValue(Application::APP_ID, 'link_preview_enabled', '1') === '1'; $searchEnabled = $this->config->getAppValue(Application::APP_ID, 'search_gifs_enabled', '1') === '1'; diff --git a/tests/unit/Settings/AdminTest.php b/tests/unit/Settings/AdminTest.php new file mode 100644 index 0000000..a092ed5 --- /dev/null +++ b/tests/unit/Settings/AdminTest.php @@ -0,0 +1,94 @@ +config = $this->createMock(IConfig::class); + $this->crypto = $this->createMock(ICrypto::class); + $this->initialState = $this->createMock(IInitialState::class); + $this->logger = $this->createMock(LoggerInterface::class); + $this->admin = new Admin($this->config, $this->crypto, $this->initialState, $this->logger); + } + + /** + * Returns the 'admin-config' array handed to the frontend. + */ + private function captureAdminConfig(): array { + $captured = []; + $this->initialState->method('provideInitialState') + ->willReturnCallback(function (string $key, $value) use (&$captured): void { + if ($key === 'admin-config') { + $captured = $value; + } + }); + $this->admin->getForm(); + return $captured; + } + + private function configReturns(string $apiKey): void { + $this->config->method('getAppValue') + ->willReturnCallback(function (string $app, string $key, string $default = '') use ($apiKey): string { + return $key === 'api_key' ? $apiKey : $default; + }); + } + + public function testApiKeyIsMaskedWhenItDecrypts(): void { + $this->configReturns('some-ciphertext'); + $this->crypto->expects($this->once()) + ->method('decrypt') + ->with('some-ciphertext') + ->willReturn('the-real-key'); + + $this->assertSame('dummyApiKey', $this->captureAdminConfig()['api_key']); + } + + public function testNoApiKeyIsReportedWhenNoneIsStored(): void { + $this->configReturns(''); + $this->crypto->expects($this->never())->method('decrypt'); + + $this->assertSame('', $this->captureAdminConfig()['api_key']); + } + + /** + * A stored value that is not valid ciphertext must not escape getForm(): + * the settings controller renders every app's section, so an exception here + * takes down the entire "Connected accounts" page rather than just ours. + */ + public function testUndecryptableApiKeyDoesNotBreakTheSettingsPage(): void { + $this->configReturns('not-actually-ciphertext'); + $this->crypto->method('decrypt') + ->willThrowException(new \Exception('Authenticated ciphertext could not be decoded.')); + $this->logger->expects($this->once())->method('warning'); + + $this->assertSame('', $this->captureAdminConfig()['api_key']); + } + + public function testSectionAndPriority(): void { + $this->assertSame('connected-accounts', $this->admin->getSection()); + $this->assertSame(10, $this->admin->getPriority()); + $this->assertSame('integration_giphy', Application::APP_ID); + } +}