Skip to content

Commit

Permalink
Merge pull request #67 from nextcloud/fix/config-handling
Browse files Browse the repository at this point in the history
feat: correct config handling, encrypt secrets
  • Loading branch information
julien-nc authored Oct 24, 2024
2 parents 660a40e + c4ad4f5 commit 4ef01b1
Show file tree
Hide file tree
Showing 7 changed files with 169 additions and 30 deletions.
4 changes: 2 additions & 2 deletions appinfo/info.xml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
<summary>Integration of Discourse forum and mailing list management system</summary>
<description><![CDATA[Discourse integration provides a dashboard widget displaying your important notifications
and the ability to find topics and posts with Nextcloud's unified search.]]></description>
<version>2.1.0</version>
<version>2.2.0</version>
<licence>agpl</licence>
<author>Julien Veyssier</author>
<namespace>Discourse</namespace>
Expand All @@ -22,7 +22,7 @@
<bugs>https://github.com/nextcloud/integration_discourse/issues</bugs>
<screenshot>https://github.com/nextcloud/integration_discourse/raw/main/img/screenshot1.jpg</screenshot>
<dependencies>
<nextcloud min-version="28" max-version="30"/>
<nextcloud min-version="28" max-version="31"/>
</dependencies>
<settings>
<personal>OCA\Discourse\Settings\Personal</personal>
Expand Down
27 changes: 19 additions & 8 deletions lib/Controller/ConfigController.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

namespace OCA\Discourse\Controller;

use OCP\AppFramework\Http\Attribute\NoAdminRequired;
use OCP\IURLGenerator;
use OCP\IConfig;
use OCP\IL10N;
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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');
Expand Down
20 changes: 15 additions & 5 deletions lib/Controller/DiscourseAPIController.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

use OCA\Discourse\Service\DiscourseAPIService;
use OCA\Discourse\AppInfo\Application;
use OCP\Security\ICrypto;

class DiscourseAPIController extends Controller {

Expand All @@ -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');
}
Expand Down
91 changes: 91 additions & 0 deletions lib/Migration/Version2200Date202410231719.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
<?php
/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

declare(strict_types=1);

namespace OCA\Discourse\Migration;

use Closure;
use OCA\Discourse\AppInfo\Application;
use OCP\DB\ISchemaWrapper;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\Migration\IOutput;
use OCP\Migration\SimpleMigrationStep;
use OCP\Security\ICrypto;

class Version2200Date202410231719 extends SimpleMigrationStep {

public function __construct(
private IDBConnection $connection,
private ICrypto $crypto,
private IConfig $config,
) {
}

/**
* @param IOutput $output
* @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
* @param array $options
*
* @return null|ISchemaWrapper
*/
public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options) {
// encrypt private_key
$privKey = $this->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;
}
}
17 changes: 12 additions & 5 deletions lib/Search/DiscourseSearchPostsProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
) {
}

/**
Expand Down Expand Up @@ -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) {
Expand Down
17 changes: 12 additions & 5 deletions lib/Search/DiscourseSearchTopicsProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
) {
}

/**
Expand Down Expand Up @@ -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) {
Expand Down
23 changes: 18 additions & 5 deletions lib/Settings/Personal.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
) {
}

/**
Expand All @@ -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');
Expand All @@ -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 === '') {
Expand All @@ -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);
Expand Down

0 comments on commit 4ef01b1

Please sign in to comment.