From 9056f2f284c7c570ce4ace86630f4e92f8e794e8 Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Tue, 30 Dec 2025 23:27:18 +0100 Subject: [PATCH 1/3] enh: make admin config values lazy, make client id and secret sensitive Signed-off-by: Julien Veyssier --- appinfo/info.xml | 2 +- lib/Controller/ConfigController.php | 19 ++++---- .../Version030200Date20251230225956.php | 48 +++++++++++++++++++ lib/Reference/ZammadReferenceProvider.php | 8 ++-- lib/Search/ZammadSearchProvider.php | 4 +- lib/Service/ZammadAPIService.php | 12 ++--- lib/Settings/Admin.php | 16 +++---- lib/Settings/Personal.php | 10 ++-- 8 files changed, 83 insertions(+), 36 deletions(-) create mode 100644 lib/Migration/Version030200Date20251230225956.php diff --git a/appinfo/info.xml b/appinfo/info.xml index c2a5a07..0cdd453 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -5,7 +5,7 @@ Integration of Zammad user support/ticketing solution - 3.1.0 + 3.2.0 agpl Julien Veyssier Zammad diff --git a/lib/Controller/ConfigController.php b/lib/Controller/ConfigController.php index 7ce2a6a..983829c 100644 --- a/lib/Controller/ConfigController.php +++ b/lib/Controller/ConfigController.php @@ -23,6 +23,7 @@ use OCP\AppFramework\Http\Attribute\PasswordConfirmationRequired; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\RedirectResponse; +use OCP\IAppConfig; use OCP\IConfig; use OCP\IL10N; use OCP\IRequest; @@ -37,6 +38,7 @@ public function __construct( string $appName, IRequest $request, private IConfig $config, + private IAppConfig $appConfig, private IURLGenerator $urlGenerator, private IL10N $l, private ICrypto $crypto, @@ -120,7 +122,7 @@ public function setAdminConfig(array $values): DataResponse { if (in_array($key, ['client_id', 'client_secret', 'oauth_instance_url'], true)) { return new DataResponse([], Http::STATUS_BAD_REQUEST); } - $this->config->setAppValue(Application::APP_ID, $key, $value); + $this->appConfig->setValueString(Application::APP_ID, $key, $value, lazy: true); } return new DataResponse([]); } @@ -135,10 +137,9 @@ public function setAdminConfig(array $values): DataResponse { public function setSensitiveAdminConfig(array $values): DataResponse { foreach ($values as $key => $value) { if (in_array($key, ['client_id', 'client_secret'], true) && $value !== '') { - $encryptedValue = $this->crypto->encrypt($value); - $this->config->setAppValue(Application::APP_ID, $key, $encryptedValue); + $this->appConfig->setValueString(Application::APP_ID, $key, $value, lazy: true, sensitive: true); } else { - $this->config->setAppValue(Application::APP_ID, $key, $value); + $this->appConfig->setValueString(Application::APP_ID, $key, $value, lazy: true); } } return new DataResponse([]); @@ -156,15 +157,13 @@ public function setSensitiveAdminConfig(array $values): DataResponse { #[NoCSRFRequired] public function oauthRedirect(string $code = '', string $state = ''): RedirectResponse { $configState = $this->config->getUserValue($this->userId, Application::APP_ID, 'oauth_state'); - $clientID = $this->config->getAppValue(Application::APP_ID, 'client_id'); - $clientID = $clientID === '' ? '' : $this->crypto->decrypt($clientID); - $clientSecret = $this->config->getAppValue(Application::APP_ID, 'client_secret'); - $clientSecret = $clientSecret === '' ? '' : $this->crypto->decrypt($clientSecret); + $clientID = $this->appConfig->getValueString(Application::APP_ID, 'client_id', lazy: true); + $clientSecret = $this->appConfig->getValueString(Application::APP_ID, 'client_secret', lazy: true); // anyway, reset state $this->config->setUserValue($this->userId, Application::APP_ID, 'oauth_state', ''); - $adminZammadOauthUrl = $this->config->getAppValue(Application::APP_ID, 'oauth_instance_url'); + $adminZammadOauthUrl = $this->appConfig->getValueString(Application::APP_ID, 'oauth_instance_url', lazy: true); if ($clientID && $clientSecret && $configState !== '' && $configState === $state) { $redirect_uri = $this->config->getUserValue($this->userId, Application::APP_ID, 'redirect_uri'); @@ -211,7 +210,7 @@ public function oauthRedirect(string $code = '', string $state = ''): RedirectRe * @throws PreConditionNotMetException */ private function storeUserInfo(): array { - $adminZammadOauthUrl = $this->config->getAppValue(Application::APP_ID, 'oauth_instance_url'); + $adminZammadOauthUrl = $this->appConfig->getValueString(Application::APP_ID, 'oauth_instance_url', lazy: true); $zammadUrl = $this->config->getUserValue($this->userId, Application::APP_ID, 'url') ?: $adminZammadOauthUrl; if (!$zammadUrl || !preg_match('/^(https?:\/\/)?[^.]+\.[^.].*/', $zammadUrl)) { diff --git a/lib/Migration/Version030200Date20251230225956.php b/lib/Migration/Version030200Date20251230225956.php new file mode 100644 index 0000000..04a5d4b --- /dev/null +++ b/lib/Migration/Version030200Date20251230225956.php @@ -0,0 +1,48 @@ +appConfig->getValueString(Application::APP_ID, $key); + if ($value !== '') { + $decryptedValue = $this->crypto->decrypt($value); + $this->appConfig->setValueString(Application::APP_ID, $key, $decryptedValue, lazy: true, sensitive: true); + } + } + + foreach (['oauth_instance_url', 'link_preview_enabled'] as $key) { + $value = $this->appConfig->getValueString(Application::APP_ID, $key); + if ($value !== '') { + $this->appConfig->setValueString(Application::APP_ID, $key, $value, lazy: true); + } + } + } +} diff --git a/lib/Reference/ZammadReferenceProvider.php b/lib/Reference/ZammadReferenceProvider.php index bb9c57c..447b420 100644 --- a/lib/Reference/ZammadReferenceProvider.php +++ b/lib/Reference/ZammadReferenceProvider.php @@ -31,6 +31,7 @@ use OCP\Collaboration\Reference\IReferenceManager; use OCP\Collaboration\Reference\ISearchableReferenceProvider; use OCP\Collaboration\Reference\Reference; +use OCP\IAppConfig; use OCP\IConfig; use OCP\IL10N; use OCP\IURLGenerator; @@ -41,6 +42,7 @@ class ZammadReferenceProvider extends ADiscoverableReferenceProvider implements public function __construct( private ZammadAPIService $zammadAPIService, private IConfig $config, + private IAppConfig $appConfig, private IReferenceManager $referenceManager, private IURLGenerator $urlGenerator, private IL10N $l10n, @@ -108,12 +110,12 @@ public function matchReference(string $referenceText): bool { return false; } } - $adminLinkPreviewEnabled = $this->config->getAppValue(Application::APP_ID, 'link_preview_enabled', '1') === '1'; + $adminLinkPreviewEnabled = $this->appConfig->getValueString(Application::APP_ID, 'link_preview_enabled', '1', lazy: true) === '1'; if (!$adminLinkPreviewEnabled) { return false; } - $adminZammadOauthUrl = $this->config->getAppValue(Application::APP_ID, 'oauth_instance_url'); + $adminZammadOauthUrl = $this->appConfig->getValueString(Application::APP_ID, 'oauth_instance_url', lazy: true); $zammadUrl = $this->config->getUserValue($this->userId, Application::APP_ID, 'url') ?: $adminZammadOauthUrl; return $this->isMatching($referenceText, $zammadUrl); @@ -123,7 +125,7 @@ public function matchReference(string $referenceText): bool { * @inheritDoc */ public function resolveReference(string $referenceText): ?IReference { - $adminZammadOauthUrl = $this->config->getAppValue(Application::APP_ID, 'oauth_instance_url'); + $adminZammadOauthUrl = $this->appConfig->getValueString(Application::APP_ID, 'oauth_instance_url', lazy: true); $zammadUrl = $this->config->getUserValue($this->userId, Application::APP_ID, 'url') ?: $adminZammadOauthUrl; if ($zammadUrl !== '') { $parts = $this->getLinkParts($zammadUrl, $referenceText); diff --git a/lib/Search/ZammadSearchProvider.php b/lib/Search/ZammadSearchProvider.php index 32c77a3..df72f3a 100644 --- a/lib/Search/ZammadSearchProvider.php +++ b/lib/Search/ZammadSearchProvider.php @@ -28,6 +28,7 @@ use OCA\Zammad\AppInfo\Application; use OCA\Zammad\Service\ZammadAPIService; use OCP\App\IAppManager; +use OCP\IAppConfig; use OCP\IConfig; use OCP\IDateTimeFormatter; use OCP\IL10N; @@ -43,6 +44,7 @@ public function __construct( private IAppManager $appManager, private IL10N $l10n, private IConfig $config, + private IAppConfig $appConfig, private IURLGenerator $urlGenerator, private IDateTimeFormatter $dateTimeFormatter, private ZammadAPIService $service, @@ -93,7 +95,7 @@ public function search(IUser $user, ISearchQuery $query): SearchResult { ? $this->urlGenerator->imagePath(Application::APP_ID, 'app.svg') : $this->urlGenerator->imagePath(Application::APP_ID, 'app-dark.svg'); - $adminZammadOauthUrl = $this->config->getAppValue(Application::APP_ID, 'oauth_instance_url'); + $adminZammadOauthUrl = $this->appConfig->getValueString(Application::APP_ID, 'oauth_instance_url', lazy: true); $zammadUrl = $this->config->getUserValue($user->getUID(), Application::APP_ID, 'url') ?: $adminZammadOauthUrl; $hasAccessToken = $this->config->getUserValue($user->getUID(), Application::APP_ID, 'token') !== ''; diff --git a/lib/Service/ZammadAPIService.php b/lib/Service/ZammadAPIService.php index 948dd77..c04b1a9 100644 --- a/lib/Service/ZammadAPIService.php +++ b/lib/Service/ZammadAPIService.php @@ -21,6 +21,7 @@ use OCP\AppFramework\Http; use OCP\Http\Client\IClient; use OCP\Http\Client\IClientService; +use OCP\IAppConfig; use OCP\ICache; use OCP\ICacheFactory; use OCP\IConfig; @@ -45,6 +46,7 @@ public function __construct( private LoggerInterface $logger, private IL10N $l10n, private IConfig $config, + private IAppConfig $appConfig, private INotificationManager $notificationManager, private ICrypto $crypto, ICacheFactory $cacheFactory, @@ -55,7 +57,7 @@ public function __construct( } public function getZammadUrl(string $userId): string { - $adminZammadOauthUrl = $this->config->getAppValue(Application::APP_ID, 'oauth_instance_url'); + $adminZammadOauthUrl = $this->appConfig->getValueString(Application::APP_ID, 'oauth_instance_url', lazy: true); return $this->config->getUserValue($userId, Application::APP_ID, 'url') ?: $adminZammadOauthUrl; } @@ -537,13 +539,11 @@ private function checkTokenExpiration(string $userId): void { } private function refreshToken(string $userId): bool { - $clientID = $this->config->getAppValue(Application::APP_ID, 'client_id'); - $clientID = $clientID === '' ? '' : $this->crypto->decrypt($clientID); - $clientSecret = $this->config->getAppValue(Application::APP_ID, 'client_secret'); - $clientSecret = $clientSecret === '' ? '' : $this->crypto->decrypt($clientSecret); + $clientID = $this->appConfig->getValueString(Application::APP_ID, 'client_id', lazy: true); + $clientSecret = $this->appConfig->getValueString(Application::APP_ID, 'client_secret', lazy: true); $refreshToken = $this->config->getUserValue($userId, Application::APP_ID, 'refresh_token'); $refreshToken = $refreshToken === '' ? '' : $this->crypto->decrypt($refreshToken); - $adminZammadOauthUrl = $this->config->getAppValue(Application::APP_ID, 'oauth_instance_url'); + $adminZammadOauthUrl = $this->appConfig->getValueString(Application::APP_ID, 'oauth_instance_url', lazy: true); if (!$refreshToken) { $this->logger->error('No Zammad refresh token found', ['app' => Application::APP_ID]); return false; diff --git a/lib/Settings/Admin.php b/lib/Settings/Admin.php index 85a1def..a47c5f9 100644 --- a/lib/Settings/Admin.php +++ b/lib/Settings/Admin.php @@ -5,17 +5,15 @@ use OCA\Zammad\AppInfo\Application; use OCP\AppFramework\Http\TemplateResponse; use OCP\AppFramework\Services\IInitialState; -use OCP\IConfig; +use OCP\IAppConfig; -use OCP\Security\ICrypto; use OCP\Settings\ISettings; class Admin implements ISettings { public function __construct( - private IConfig $config, + private IAppConfig $appConfig, private IInitialState $initialStateService, - private ICrypto $crypto, ) { } @@ -23,13 +21,11 @@ public function __construct( * @return TemplateResponse */ public function getForm(): TemplateResponse { - $clientID = $this->config->getAppValue(Application::APP_ID, 'client_id'); - $clientID = $clientID === '' ? '' : $this->crypto->decrypt($clientID); - $clientSecret = $this->config->getAppValue(Application::APP_ID, 'client_secret'); - $clientSecret = $clientSecret === '' ? '' : $this->crypto->decrypt($clientSecret); + $clientID = $this->appConfig->getValueString(Application::APP_ID, 'client_id', lazy: true); + $clientSecret = $this->appConfig->getValueString(Application::APP_ID, 'client_secret', lazy: true); - $oauthUrl = $this->config->getAppValue(Application::APP_ID, 'oauth_instance_url'); - $adminLinkPreviewEnabled = $this->config->getAppValue(Application::APP_ID, 'link_preview_enabled', '1') === '1'; + $oauthUrl = $this->appConfig->getValueString(Application::APP_ID, 'oauth_instance_url', lazy: true); + $adminLinkPreviewEnabled = $this->appConfig->getValueString(Application::APP_ID, 'link_preview_enabled', '1', lazy: true) === '1'; $adminConfig = [ 'client_id' => $clientID, diff --git a/lib/Settings/Personal.php b/lib/Settings/Personal.php index e2f2c70..323961c 100644 --- a/lib/Settings/Personal.php +++ b/lib/Settings/Personal.php @@ -5,6 +5,7 @@ use OCA\Zammad\AppInfo\Application; use OCP\AppFramework\Http\TemplateResponse; use OCP\AppFramework\Services\IInitialState; +use OCP\IAppConfig; use OCP\IConfig; use OCP\Security\ICrypto; @@ -14,6 +15,7 @@ class Personal implements ISettings { public function __construct( private IConfig $config, + private IAppConfig $appConfig, private IInitialState $initialStateService, private ICrypto $crypto, private ?string $userId, @@ -25,11 +27,9 @@ public function __construct( */ public function getForm(): TemplateResponse { // for OAuth - $clientID = $this->config->getAppValue(Application::APP_ID, 'client_id'); - $clientID = $clientID === '' ? '' : $this->crypto->decrypt($clientID); - $clientSecret = $this->config->getAppValue(Application::APP_ID, 'client_secret'); - $clientSecret = $clientSecret === '' ? '' : $this->crypto->decrypt($clientSecret); - $adminOauthUrl = $this->config->getAppValue(Application::APP_ID, 'oauth_instance_url'); + $clientID = $this->appConfig->getValueString(Application::APP_ID, 'client_id', lazy: true); + $clientSecret = $this->appConfig->getValueString(Application::APP_ID, 'client_secret', lazy: true); + $adminOauthUrl = $this->appConfig->getValueString(Application::APP_ID, 'oauth_instance_url', lazy: true); $token = $this->config->getUserValue($this->userId, Application::APP_ID, 'token'); $token = $token === '' ? '' : $this->crypto->decrypt($token); From e941b3890ac4471ec0711dbfeec1049ae988a7cb Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Tue, 30 Dec 2025 23:31:38 +0100 Subject: [PATCH 2/3] fix: avoid using Php 8.5 in psalm action, not supported by psalm for now Signed-off-by: Julien Veyssier --- .github/workflows/psalm-matrix.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/psalm-matrix.yml b/.github/workflows/psalm-matrix.yml index cdf2dba..bfca91f 100644 --- a/.github/workflows/psalm-matrix.yml +++ b/.github/workflows/psalm-matrix.yml @@ -45,10 +45,12 @@ jobs: - name: Checkout uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - - name: Set up php${{ matrix.php-versions }} + #- name: Set up php${{ matrix.php-versions }} + - name: Set up php8.2 uses: shivammathur/setup-php@bf6b4fbd49ca58e4608c9c89fba0b8d90bd2a39f # v2.35.5 with: - php-version: ${{ matrix.php-versions }} + #php-version: ${{ matrix.php-versions }} + php-version: 8.2 extensions: bz2, ctype, curl, dom, fileinfo, gd, iconv, intl, json, libxml, mbstring, openssl, pcntl, posix, session, simplexml, xmlreader, xmlwriter, zip, zlib, sqlite, pdo_sqlite coverage: none ini-file: development From c101b0fab42f515b82731d5c5491906632966353 Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Tue, 30 Dec 2025 23:41:04 +0100 Subject: [PATCH 3/3] fix: disable ensureOverrideAttribute in psalm config Signed-off-by: Julien Veyssier --- .github/workflows/psalm-matrix.yml | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/.github/workflows/psalm-matrix.yml b/.github/workflows/psalm-matrix.yml index bfca91f..cdf2dba 100644 --- a/.github/workflows/psalm-matrix.yml +++ b/.github/workflows/psalm-matrix.yml @@ -45,12 +45,10 @@ jobs: - name: Checkout uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - #- name: Set up php${{ matrix.php-versions }} - - name: Set up php8.2 + - name: Set up php${{ matrix.php-versions }} uses: shivammathur/setup-php@bf6b4fbd49ca58e4608c9c89fba0b8d90bd2a39f # v2.35.5 with: - #php-version: ${{ matrix.php-versions }} - php-version: 8.2 + php-version: ${{ matrix.php-versions }} extensions: bz2, ctype, curl, dom, fileinfo, gd, iconv, intl, json, libxml, mbstring, openssl, pcntl, posix, session, simplexml, xmlreader, xmlwriter, zip, zlib, sqlite, pdo_sqlite coverage: none ini-file: development