From 6931c8bd9aef19355ea41240759baefa1cf60fce Mon Sep 17 00:00:00 2001 From: Andrey Borysenko Date: Thu, 24 Oct 2024 00:20:53 +0300 Subject: [PATCH 1/2] feat: correct config handling, encrypt secrets Signed-off-by: Andrey Borysenko --- appinfo/info.xml | 4 +-- lib/Controller/ConfigController.php | 27 ++++++++++++++------ lib/Controller/DiscourseAPIController.php | 20 +++++++++++---- lib/Search/DiscourseSearchPostsProvider.php | 17 ++++++++---- lib/Search/DiscourseSearchTopicsProvider.php | 17 ++++++++---- lib/Settings/Personal.php | 23 +++++++++++++---- 6 files changed, 78 insertions(+), 30 deletions(-) diff --git a/appinfo/info.xml b/appinfo/info.xml index ee9559c..d344d2a 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -9,7 +9,7 @@ Integration of Discourse forum and mailing list management system - 2.1.0 + 2.2.0 agpl Julien Veyssier Discourse @@ -22,7 +22,7 @@ https://github.com/nextcloud/integration_discourse/issues https://github.com/nextcloud/integration_discourse/raw/main/img/screenshot1.jpg - + OCA\Discourse\Settings\Personal diff --git a/lib/Controller/ConfigController.php b/lib/Controller/ConfigController.php index 566b657..b676fa2 100644 --- a/lib/Controller/ConfigController.php +++ b/lib/Controller/ConfigController.php @@ -6,6 +6,7 @@ namespace OCA\Discourse\Controller; +use OCP\AppFramework\Http\Attribute\NoAdminRequired; use OCP\IURLGenerator; use OCP\IConfig; use OCP\IL10N; @@ -20,28 +21,32 @@ require_once __DIR__ . '/../../vendor/autoload.php'; use OCP\PreConditionNotMetException; +use OCP\Security\ICrypto; use phpseclib\Crypt\RSA; class ConfigController extends Controller { - public function __construct(string $appName, - IRequest $request, - private IConfig $config, - private IURLGenerator $urlGenerator, - private IL10N $l, - private DiscourseAPIService $discourseAPIService, - private ?string $userId) { + public function __construct( + string $appName, + IRequest $request, + private IConfig $config, + private ICrypto $crypto, + private IURLGenerator $urlGenerator, + private IL10N $l, + private DiscourseAPIService $discourseAPIService, + private ?string $userId + ) { parent::__construct($appName, $request); } /** * set config values - * @NoAdminRequired * * @param array $values * @return DataResponse * @throws PreConditionNotMetException */ + #[NoAdminRequired] public function setConfig(array $values): DataResponse { foreach ($values as $key => $value) { $this->config->setUserValue($this->userId, Application::APP_ID, $key, $value); @@ -107,6 +112,9 @@ public function oauthRedirect(string $payload = ''): RedirectResponse { $configNonce = $this->config->getUserValue($this->userId, Application::APP_ID, 'nonce'); // decrypt payload $privKey = $this->config->getAppValue(Application::APP_ID, 'private_key'); + if ($privKey !== '') { + $privKey = $this->crypto->decrypt($privKey); + } $decPayload = base64_decode($payload); $rsa = new RSA(); $rsa->setEncryptionMode(RSA::ENCRYPTION_PKCS1); @@ -120,6 +128,9 @@ public function oauthRedirect(string $payload = ''): RedirectResponse { if (is_array($payloadArray) && $configNonce !== '' && $configNonce === $payloadArray['nonce']) { if (isset($payloadArray['key'])) { $accessToken = $payloadArray['key']; + if ($accessToken !== '') { + $accessToken = $this->crypto->encrypt($accessToken); + } $this->config->setUserValue($this->userId, Application::APP_ID, 'token', $accessToken); // get user info $url = $this->config->getUserValue($this->userId, Application::APP_ID, 'url'); diff --git a/lib/Controller/DiscourseAPIController.php b/lib/Controller/DiscourseAPIController.php index 6920504..c48764a 100644 --- a/lib/Controller/DiscourseAPIController.php +++ b/lib/Controller/DiscourseAPIController.php @@ -15,6 +15,7 @@ use OCA\Discourse\Service\DiscourseAPIService; use OCA\Discourse\AppInfo\Application; +use OCP\Security\ICrypto; class DiscourseAPIController extends Controller { @@ -23,14 +24,23 @@ class DiscourseAPIController extends Controller { private string $discourseUrl; private string $discourseUsername; - public function __construct(string $appName, - IRequest $request, - private IConfig $config, - private DiscourseAPIService $discourseAPIService, - private ?string $userId) { + public function __construct( + string $appName, + IRequest $request, + private IConfig $config, + ICrypto $crypto, + private DiscourseAPIService $discourseAPIService, + private ?string $userId + ) { parent::__construct($appName, $request); $this->accessToken = $this->config->getUserValue($this->userId, Application::APP_ID, 'token'); + if ($this->accessToken !== '') { + $this->accessToken = $crypto->decrypt($this->accessToken); + } $this->clientID = $this->config->getUserValue($this->userId, Application::APP_ID, 'client_id'); + if ($this->clientID !== '') { + $this->clientID = $crypto->decrypt($this->clientID); + } $this->discourseUrl = $this->config->getUserValue($this->userId, Application::APP_ID, 'url'); $this->discourseUsername = $this->config->getUserValue($this->userId, Application::APP_ID, 'user_name'); } diff --git a/lib/Search/DiscourseSearchPostsProvider.php b/lib/Search/DiscourseSearchPostsProvider.php index 7d9ea8e..44d81d7 100644 --- a/lib/Search/DiscourseSearchPostsProvider.php +++ b/lib/Search/DiscourseSearchPostsProvider.php @@ -19,14 +19,18 @@ use OCP\Search\ISearchQuery; use OCP\Search\SearchResult; use OCP\Search\SearchResultEntry; +use OCP\Security\ICrypto; class DiscourseSearchPostsProvider implements IProvider { - public function __construct(private IAppManager $appManager, - private IL10N $l10n, - private IConfig $config, - private IURLGenerator $urlGenerator, - private DiscourseAPIService $discourseAPIService) { + public function __construct( + private IAppManager $appManager, + private IL10N $l10n, + private IConfig $config, + private ICrypto $crypto, + private IURLGenerator $urlGenerator, + private DiscourseAPIService $discourseAPIService + ) { } /** @@ -75,6 +79,9 @@ public function search(IUser $user, ISearchQuery $query): SearchResult { $discourseUrl = $this->config->getUserValue($user->getUID(), Application::APP_ID, 'url'); $accessToken = $this->config->getUserValue($user->getUID(), Application::APP_ID, 'token'); + if ($accessToken !== '') { + $accessToken = $this->crypto->decrypt($accessToken); + } $searchPostsEnabled = $this->config->getUserValue($user->getUID(), Application::APP_ID, 'search_posts_enabled', '0') === '1'; if ($accessToken === '' || !$searchPostsEnabled) { diff --git a/lib/Search/DiscourseSearchTopicsProvider.php b/lib/Search/DiscourseSearchTopicsProvider.php index 9ae9c6c..38af433 100644 --- a/lib/Search/DiscourseSearchTopicsProvider.php +++ b/lib/Search/DiscourseSearchTopicsProvider.php @@ -19,14 +19,18 @@ use OCP\Search\ISearchQuery; use OCP\Search\SearchResult; use OCP\Search\SearchResultEntry; +use OCP\Security\ICrypto; class DiscourseSearchTopicsProvider implements IProvider { - public function __construct(private IAppManager $appManager, - private IL10N $l10n, - private IConfig $config, - private IURLGenerator $urlGenerator, - private DiscourseAPIService $discourseAPIService) { + public function __construct( + private IAppManager $appManager, + private IL10N $l10n, + private IConfig $config, + private ICrypto $crypto, + private IURLGenerator $urlGenerator, + private DiscourseAPIService $discourseAPIService + ) { } /** @@ -70,6 +74,9 @@ public function search(IUser $user, ISearchQuery $query): SearchResult { $discourseUrl = $this->config->getUserValue($user->getUID(), Application::APP_ID, 'url'); $accessToken = $this->config->getUserValue($user->getUID(), Application::APP_ID, 'token'); + if ($accessToken !== '') { + $accessToken = $this->crypto->decrypt($accessToken); + } $searchTopicsEnabled = $this->config->getUserValue($user->getUID(), Application::APP_ID, 'search_topics_enabled', '0') === '1'; if ($accessToken === '' || !$searchTopicsEnabled) { diff --git a/lib/Settings/Personal.php b/lib/Settings/Personal.php index f700734..f113cfb 100644 --- a/lib/Settings/Personal.php +++ b/lib/Settings/Personal.php @@ -10,6 +10,7 @@ use OCP\AppFramework\Services\IInitialState; use OCP\IConfig; use OCP\PreConditionNotMetException; +use OCP\Security\ICrypto; use OCP\Settings\ISettings; use OCA\Discourse\AppInfo\Application; @@ -19,9 +20,12 @@ class Personal implements ISettings { - public function __construct(private IConfig $config, - private IInitialState $initialStateService, - private ?string $userId) { + public function __construct( + private IConfig $config, + private ICrypto $crypto, + private IInitialState $initialStateService, + private ?string $userId + ) { } /** @@ -30,6 +34,9 @@ public function __construct(private IConfig $config, */ public function getForm(): TemplateResponse { $token = $this->config->getUserValue($this->userId, Application::APP_ID, 'token'); + if ($token !== '') { + $token = $this->crypto->decrypt($token); + } $url = $this->config->getUserValue($this->userId, Application::APP_ID, 'url'); $userName = $this->config->getUserValue($this->userId, Application::APP_ID, 'user_name'); $navigationEnabled = $this->config->getUserValue($this->userId, Application::APP_ID, 'navigation_enabled', '0'); @@ -38,13 +45,19 @@ public function getForm(): TemplateResponse { // for OAuth $clientID = $this->config->getUserValue($this->userId, Application::APP_ID, 'client_id'); + if ($clientID !== '') { + $clientID = $this->crypto->decrypt($clientID); + } $pubKey = $this->config->getAppValue(Application::APP_ID, 'public_key'); $privKey = $this->config->getAppValue(Application::APP_ID, 'private_key'); + if ($privKey !== '') { + $privKey = $this->crypto->decrypt($privKey); + } if ($clientID === '') { // random string of 32 chars length $permitted_chars = '0123456789abcdefghijklmnopqrstuvwxyz'; - $clientID = substr(str_shuffle($permitted_chars), 0, 32); + $clientID = $this->crypto->encrypt(substr(str_shuffle($permitted_chars), 0, 32)); $this->config->setUserValue($this->userId, Application::APP_ID, 'client_id', $clientID); } if ($pubKey === '' || $privKey === '') { @@ -53,7 +66,7 @@ public function getForm(): TemplateResponse { $rsa->setPublicKeyFormat(RSA::PUBLIC_FORMAT_PKCS1); $keys = $rsa->createKey(2048); $pubKey = $keys['publickey']; - $privKey = $keys['privatekey']; + $privKey = $this->crypto->encrypt($keys['privatekey']); $this->config->setAppValue(Application::APP_ID, 'public_key', $pubKey); $this->config->setAppValue(Application::APP_ID, 'private_key', $privKey); From c4ad4f5c0e189701831b6d49a8249679c63d4303 Mon Sep 17 00:00:00 2001 From: Andrey Borysenko Date: Thu, 24 Oct 2024 13:29:24 +0300 Subject: [PATCH 2/2] fix: add missing migration Signed-off-by: Andrey Borysenko --- lib/Migration/Version2200Date202410231719.php | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 lib/Migration/Version2200Date202410231719.php diff --git a/lib/Migration/Version2200Date202410231719.php b/lib/Migration/Version2200Date202410231719.php new file mode 100644 index 0000000..e8f1a1b --- /dev/null +++ b/lib/Migration/Version2200Date202410231719.php @@ -0,0 +1,91 @@ +config->getAppValue(Application::APP_ID, 'private_key'); + if ($privKey !== '') { + $privKey = $this->crypto->encrypt($privKey); + $this->config->setAppValue(Application::APP_ID, 'private_key', $privKey); + } + + // encrypt user tokens and client_ids + $qbUpdate = $this->connection->getQueryBuilder(); + $qbUpdate->update('preferences') + ->set('configvalue', $qbUpdate->createParameter('updateValue')) + ->where( + $qbUpdate->expr()->eq('appid', $qbUpdate->createNamedParameter(Application::APP_ID, IQueryBuilder::PARAM_STR)) + ) + ->andWhere( + $qbUpdate->expr()->eq('configkey', $qbUpdate->createParameter('updateConfigKey')) + ); + $qbUpdate->andWhere( + $qbUpdate->expr()->eq('userid', $qbUpdate->createParameter('updateUserId')) + ); + + $qbSelect = $this->connection->getQueryBuilder(); + $qbSelect->select(['userid', 'configvalue', 'configkey']) + ->from('preferences') + ->where( + $qbSelect->expr()->eq('appid', $qbSelect->createNamedParameter(Application::APP_ID, IQueryBuilder::PARAM_STR)) + ); + + $or = $qbSelect->expr()->orx(); + $or->add($qbSelect->expr()->eq('configkey', $qbSelect->createNamedParameter('token', IQueryBuilder::PARAM_STR))); + $or->add($qbSelect->expr()->eq('configkey', $qbSelect->createNamedParameter('client_id', IQueryBuilder::PARAM_STR))); + $qbSelect->andWhere($or); + + $qbSelect->andWhere( + $qbSelect->expr()->nonEmptyString('configvalue') + ) + ->andWhere( + $qbSelect->expr()->isNotNull('configvalue') + ); + $req = $qbSelect->executeQuery(); + while ($row = $req->fetch()) { + $userId = $row['userid']; + $configKey = $row['configkey']; + $storedClearValue = $row['configvalue']; + $encryptedValue = $this->crypto->encrypt($storedClearValue); + $qbUpdate->setParameter('updateUserId', $userId, IQueryBuilder::PARAM_STR); + $qbUpdate->setParameter('updateConfigKey', $configKey, IQueryBuilder::PARAM_STR); + $qbUpdate->setParameter('updateValue', $encryptedValue, IQueryBuilder::PARAM_STR); + $qbUpdate->executeStatement(); + } + $req->closeCursor(); + return null; + } +}