Encrypt oauth client id/secret and user tokens

appinfo/info.xml

@@ -5,7 +5,7 @@
     <summary>Integration of GitLab software development management service</summary>
     <description><![CDATA[GitLab integration provides a dashboard widget displaying your most important notifications
     and a unified search provider for repositories, issues and merge requests.]]></description>
-    <version>3.1.2</version>
+    <version>3.1.3</version>
     <licence>agpl</licence>
     <author>Julien Veyssier</author>
     <namespace>Gitlab</namespace>

lib/Controller/ConfigController.php

@@ -71,7 +71,7 @@ public function addAccount(string $url, string $token) {
 		$account = new GitlabAccount();
 		$account->setUserId($this->userId);
 		$account->setUrl($url);
-		$account->setToken($token);
+		$account->setEncryptedToken($token);
 		$account->setTokenType('personal');
 
 		try {
@@ -197,9 +197,9 @@ public function oauthRedirect(string $code = '', string $state = ''): RedirectRe
 				$account = new GitlabAccount();
 				$account->setUserId($this->userId);
 				$account->setUrl($adminOauthUrl);
-				$account->setToken($result['access_token']);
+				$account->setEncryptedToken($result['access_token']);
 				$account->setTokenType('oauth');
-				$account->setRefreshToken($result['refresh_token'] ?? '');
+				$account->setEncryptedRefreshToken($result['refresh_token'] ?? '');
 				if (isset($result['expires_in'])) {
 					$nowTs = (new Datetime())->getTimestamp();
 					$expiresAt = $nowTs + (int)$result['expires_in'];

lib/Controller/GitlabAPIController.php

@@ -48,7 +48,7 @@ public function __construct(
 	public function getUserAvatar(int $accountId, int $userId) {
 		try {
 			$account = $this->accountMapper->findById($this->userId, $accountId);
-			if ($account->getToken() === '') {
+			if ($account->getClearToken() === '') {
 				return new DataResponse('', 400);
 			}
 
@@ -83,7 +83,7 @@ public function getUserAvatar(int $accountId, int $userId) {
 	public function getProjectAvatar(int $accountId, int $projectId) {
 		try {
 			$account = $this->accountMapper->findById($this->userId, $accountId);
-			if ($account->getToken() === '') {
+			if ($account->getClearToken() === '') {
 				return new DataResponse('', 400);
 			}
 
@@ -117,7 +117,7 @@ public function getProjectAvatar(int $accountId, int $projectId) {
 	public function getTodos(int $accountId, ?string $since = null): DataResponse {
 		try {
 			$account = $this->accountMapper->findById($this->userId, $accountId);
-			if ($account->getToken() === '') {
+			if ($account->getClearToken() === '') {
 				return new DataResponse('', 400);
 			}
 

lib/Db/GitlabAccount.php

@@ -6,6 +6,7 @@
 
 use JsonSerializable;
 use OCP\AppFramework\Db\Entity;
+use OCP\Security\ICrypto;
 
 /**
  * @method void setUserId(string $userId)
@@ -25,14 +26,29 @@
  * @method void setUserInfoDisplayName(string $userInfoDisplayName)
  */
 class GitlabAccount extends Entity implements JsonSerializable {
-	protected string $userId = '';
-	protected string $url = '';
-	protected string $token = '';
-	protected string $tokenType = '';
-	protected ?int $tokenExpiresAt = null;
-	protected ?string $refreshToken = null;
-	protected ?string $userInfoName = null;
-	protected ?string $userInfoDisplayName = null;
+	protected $userId;
+	protected $url;
+	protected $token;
+	protected $tokenType;
+	protected $tokenExpiresAt;
+	protected $refreshToken;
+	protected $userInfoName;
+	protected $userInfoDisplayName;
+
+	private ICrypto $crypto;
+
+	public function __construct() {
+		$this->addType('id', 'integer');
+		$this->addType('url', 'string');
+		$this->addType('token', 'string');
+		$this->addType('token_type', 'string');
+		$this->addType('token_expires_at', 'integer');
+		$this->addType('refresh_token', 'string');
+		$this->addType('user_info_name', 'string');
+		$this->addType('user_info_display_name', 'string');
+
+		$this->crypto = \OC::$server->get(ICrypto::class);
+	}
 
 	public function jsonSerialize(): array {
 		return [
@@ -46,4 +62,34 @@ public function jsonSerialize(): array {
 			'userInfoDisplayName' => $this->userInfoDisplayName,
 		];
 	}
+
+	public function getClearToken(): string {
+		if (is_string($this->token) && $this->token !== '') {
+			return $this->crypto->decrypt($this->token);
+		}
+		return $this->token;
+	}
+
+	public function setEncryptedToken(string $token): void {
+		if ($token !== '') {
+			$this->setter('token', [$this->crypto->encrypt($token)]);
+		} else {
+			$this->setter('token', [$token]);
+		}
+	}
+
+	public function getClearRefreshToken(): ?string {
+		if (is_string($this->refreshToken) && $this->refreshToken !== '') {
+			return $this->crypto->decrypt($this->refreshToken);
+		}
+		return $this->refreshToken;
+	}
+
+	public function setEncryptedRefreshToken(?string $refreshToken): void {
+		if (is_string($refreshToken) && $refreshToken !== '') {
+			$this->setter('refreshToken', [$this->crypto->encrypt($refreshToken)]);
+		} else {
+			$this->setter('refreshToken', [$refreshToken]);
+		}
+	}
 }

lib/Migration/MultiAccountRepairStep.php

@@ -30,7 +30,7 @@ public function run(IOutput $output) {
 				$account = new GitlabAccount();
 				$account->setUserId($user->getUID());
 				$account->setUrl($this->config->getUserUrl($userId));
-				$account->setToken($this->config->getUserToken($userId));
+				$account->setEncryptedToken($this->config->getUserToken($userId));
 				$account->setTokenType($this->config->getUserTokenType($userId));
 				if (!$account->getTokenType()) {
 					if ($this->config->hasUserRefreshToken($userId)) {
@@ -43,7 +43,7 @@ public function run(IOutput $output) {
 					$account->setTokenExpiresAt($this->config->getUserTokenExpiresAt($userId));
 				}
 				if ($this->config->hasUserRefreshToken($userId)) {
-					$account->setRefreshToken($this->config->getUserRefreshToken($userId));
+					$account->setEncryptedRefreshToken($this->config->getUserRefreshToken($userId));
 				}
 				$account->setUserInfoName($this->config->getUserName($userId));
 				$account->setUserInfoDisplayName($this->config->getUserDisplayName($userId));

lib/Migration/Version030103Date20241017150114.php

@@ -0,0 +1,97 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2020 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+namespace OCA\Gitlab\Migration;
+
+use Closure;
+use OCA\Gitlab\AppInfo\Application;
+use OCP\DB\QueryBuilder\IQueryBuilder;
+use OCP\IConfig;
+use OCP\IDBConnection;
+use OCP\Migration\IOutput;
+use OCP\Migration\SimpleMigrationStep;
+use OCP\Security\ICrypto;
+
+class Version030103Date20241017150114 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
+	 */
+	public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options) {
+		// app config
+		foreach (['client_id', 'client_secret'] as $key) {
+			$value = $this->config->getAppValue(Application::APP_ID, $key);
+			if ($value !== '') {
+				$encryptedValue = $this->crypto->encrypt($value);
+				$this->config->setAppValue(Application::APP_ID, $key, $encryptedValue);
+			}
+		}
+
+		// ----------- user tokens
+		$qbUpdate = $this->connection->getQueryBuilder();
+		$qbUpdate->update('gitlab_accounts')
+			->set('token', $qbUpdate->createParameter('updateToken'))
+			->where(
+				$qbUpdate->expr()->eq('id', $qbUpdate->createParameter('updateAccountId'))
+			);
+
+		$qbSelect = $this->connection->getQueryBuilder();
+		$qbSelect->select('id', 'token')
+			->from('gitlab_accounts')
+			->where(
+				$qbSelect->expr()->nonEmptyString('token')
+			);
+		$req = $qbSelect->executeQuery();
+		while ($row = $req->fetch()) {
+			$accountId = (int)$row['id'];
+			$storedClearRefreshToken = $row['token'];
+			$encryptedToken = $this->crypto->encrypt($storedClearRefreshToken);
+			$qbUpdate->setParameter('updateAccountId', $accountId, IQueryBuilder::PARAM_INT);
+			$qbUpdate->setParameter('updateToken', $encryptedToken, IQueryBuilder::PARAM_STR);
+			$qbUpdate->executeStatement();
+		}
+		$req->closeCursor();
+
+		// ----------- user refresh tokens
+		$qbUpdate = $this->connection->getQueryBuilder();
+		$qbUpdate->update('gitlab_accounts')
+			->set('refresh_token', $qbUpdate->createParameter('updateRefreshToken'))
+			->where(
+				$qbUpdate->expr()->eq('id', $qbUpdate->createParameter('updateAccountId'))
+			);
+
+		$qbSelect = $this->connection->getQueryBuilder();
+		$qbSelect->select('id', 'refresh_token')
+			->from('gitlab_accounts')
+			->where(
+				$qbSelect->expr()->nonEmptyString('refresh_token')
+			)
+			->andWhere(
+				$qbSelect->expr()->isNotNull('refresh_token')
+			);
+		$req = $qbSelect->executeQuery();
+		while ($row = $req->fetch()) {
+			$accountId = (int)$row['id'];
+			$storedClearRefreshToken = $row['refresh_token'];
+			$encryptedToken = $this->crypto->encrypt($storedClearRefreshToken);
+			$qbUpdate->setParameter('updateAccountId', $accountId, IQueryBuilder::PARAM_INT);
+			$qbUpdate->setParameter('updateRefreshToken', $encryptedToken, IQueryBuilder::PARAM_STR);
+			$qbUpdate->executeStatement();
+		}
+		$req->closeCursor();
+	}
+}

lib/Search/GitlabSearchIssuesProvider.php

@@ -102,7 +102,7 @@ public function search(IUser $user, ISearchQuery $query): SearchResult {
 
 		$accounts = $this->accountMapper->find($user->getUID());
 		foreach ($accounts as $account) {
-			$accessToken = $account->getToken();
+			$accessToken = $account->getClearToken();
 			if ($accessToken === '') {
 				continue;
 			}

lib/Search/GitlabSearchMergeRequestsProvider.php

@@ -102,7 +102,7 @@ public function search(IUser $user, ISearchQuery $query): SearchResult {
 
 		$accounts = $this->accountMapper->find($user->getUID());
 		foreach ($accounts as $account) {
-			$accessToken = $account->getToken();
+			$accessToken = $account->getClearToken();
 			if ($accessToken === '') {
 				continue;
 			}

lib/Search/GitlabSearchReposProvider.php

@@ -102,7 +102,7 @@ public function search(IUser $user, ISearchQuery $query): SearchResult {
 
 		$accounts = $this->accountMapper->find($user->getUID());
 		foreach ($accounts as $account) {
-			$accessToken = $account->getToken();
+			$accessToken = $account->getClearToken();
 			if ($accessToken === '') {
 				continue;
 			}

lib/Service/ConfigService.php

@@ -7,31 +7,50 @@
 use OCA\Gitlab\AppInfo\Application;
 use OCP\IConfig;
 use OCP\PreConditionNotMetException;
+use OCP\Security\ICrypto;
 
 class ConfigService {
 	public function __construct(
 		private IConfig $config,
+		private ICrypto $crypto,
 	) {
 	}
 
+	public function getClearAppValue(string $key): string {
+		$value = $this->config->getAppValue(Application::APP_ID, $key);
+		if ($value === '') {
+			return $value;
+		}
+		return $this->crypto->decrypt($value);
+	}
+
+	public function setEncryptedAppValue(string $key, string $value): void {
+		if ($value === '') {
+			$this->config->setAppValue(Application::APP_ID, $key, $value);
+		} else {
+			$encryptedValue = $this->crypto->encrypt($value);
+			$this->config->setAppValue(Application::APP_ID, $key, $encryptedValue);
+		}
+	}
+
 	public function getAdminClientId(): string {
-		return $this->config->getAppValue(Application::APP_ID, 'client_id');
+		return $this->getClearAppValue('client_id');
 	}
 
 	public function setAdminClientId(string $clientId): void {
-		$this->config->setAppValue(Application::APP_ID, 'client_id', $clientId);
+		$this->setEncryptedAppValue('client_id', $clientId);
 	}
 
 	public function hasAdminClientSecret(): bool {
 		return $this->getAdminClientSecret() !== '';
 	}
 
 	public function getAdminClientSecret(): string {
-		return $this->config->getAppValue(Application::APP_ID, 'client_secret');
+		return $this->getClearAppValue('client_secret');
 	}
 
 	public function setAdminClientSecret(string $clientSecret): void {
-		$this->config->setAppValue(Application::APP_ID, 'client_secret', $clientSecret);
+		$this->setEncryptedAppValue('client_secret', $clientSecret);
 	}
 
 	public function getAdminOauthUrl(): string {

lib/Service/GitlabAPIService.php

@@ -243,7 +243,7 @@ public function request(?GitlabAccount $account, string $baseUrl, string $endPoi
 
 			// try anonymous request if no user (public page) or user not connected to a gitlab account
 			if ($account !== null) {
-				$accessToken = $account->getToken();
+				$accessToken = $account->getClearToken();
 				if ($accessToken !== '') {
 					$options['headers']['Authorization'] = 'Bearer ' . $accessToken;
 				}
@@ -301,7 +301,7 @@ public function request(?GitlabAccount $account, string $baseUrl, string $endPoi
 	}
 
 	private function checkTokenExpiration(GitlabAccount $account): void {
-		if ($account->getRefreshToken() && $account->getTokenExpiresAt()) {
+		if ($account->getClearRefreshToken() && $account->getTokenExpiresAt()) {
 			$nowTs = (new DateTime())->getTimestamp();
 			// if token expires in less than a minute or is already expired
 			if ($nowTs > $account->getTokenExpiresAt() - 60) {
@@ -312,7 +312,7 @@ private function checkTokenExpiration(GitlabAccount $account): void {
 
 	private function refreshToken(GitlabAccount $account): bool {
 		$adminOauthUrl = $this->config->getAdminOauthUrl();
-		$refreshToken = $account->getRefreshToken();
+		$refreshToken = $account->getClearRefreshToken();
 		if (!$refreshToken) {
 			$this->logger->error('No GitLab refresh token found', ['app' => Application::APP_ID]);
 			return false;
@@ -329,8 +329,8 @@ private function refreshToken(GitlabAccount $account): bool {
 			$accessToken = $result['access_token'];
 			$refreshToken = $result['refresh_token'];
 			$account->setUrl($adminOauthUrl);
-			$account->setToken($accessToken);
-			$account->setRefreshToken($refreshToken);
+			$account->setEncryptedToken($accessToken);
+			$account->setEncryptedRefreshToken($refreshToken);
 			if (isset($result['expires_in'])) {
 				$nowTs = (new DateTime())->getTimestamp();
 				$expiresAt = $nowTs + (int) $result['expires_in'];
@@ -361,7 +361,7 @@ public function revokeOauthToken(GitlabAccount $account): array {
 				'body' => json_encode([
 					'client_id' => $this->config->getAdminClientId(),
 					'client_secret' => $this->config->getAdminClientSecret(),
-					'token' => $account->getToken(),
+					'token' => $account->getClearToken(),
 				]),
 			];
 

psalm.xml

@@ -30,6 +30,7 @@
 				<referencedClass name="GuzzleHttp\Exception\ServerException" />
 				<referencedClass name="GuzzleHttp\Exception\ClientException" />
 				<referencedClass name="GuzzleHttp\Exception\ConnectException" />
+				<referencedClass name="OC" />
 			</errorLevel>
 		</UndefinedClass>
 		<UndefinedDocblockClass>

tests/unit/Controller/GitlabAPIControllerTest.php

@@ -62,7 +62,7 @@ protected function setUp(): void {
 
 		$account = new GitlabAccount();
 		$account->setUrl('https://gitlab.com');
-		$account->setToken(self::API_TOKEN);
+		$account->setEncryptedToken(self::API_TOKEN);
 
 		$accountMapper = $this->createMock(GitlabAccountMapper::class);
 		$accountMapper->method('findById')->willReturn($account);