Encrypt tokens, client id and client secret

.github/workflows/phpunit-mysql.yml

@@ -43,7 +43,7 @@ jobs:
 
     services:
       mysql:
-        image: ghcr.io/nextcloud/continuous-integration-mariadb-10.6:latest
+        image: mariadb:10.5
         ports:
           - 4444:3306/tcp
         env:

.github/workflows/psalm.yml

@@ -38,7 +38,7 @@ jobs:
       # do not stop on another job's failure
       fail-fast: false
       matrix:
-        ocp-version: ['^26', '^27', '^28', '^29', '^30', 'dev-master']
+        ocp-version: ['^27', '^28', '^29', '^30', 'dev-master']
         php-version: ['8.0', '8.1', '8.2', '8.3']
 
 

appinfo/info.xml

@@ -10,7 +10,7 @@
     <description><![CDATA[GitHub integration provides a dashboard widget displaying your most important notifications
     and a unified search provider for repositories, issues and pull requests. It also provides a link reference provider
     to render links to issues, pull requests and comments in Talk and Text.]]></description>
-    <version>3.0.0</version>
+    <version>3.0.1</version>
     <licence>agpl</licence>
     <author>Julien Veyssier</author>
     <namespace>Github</namespace>
@@ -23,7 +23,7 @@
     <bugs>https://github.com/nextcloud/integration_github/issues</bugs>
     <screenshot>https://github.com/nextcloud/integration_github/raw/main/img/screenshot1.jpg</screenshot>
     <dependencies>
-        <nextcloud min-version="26" max-version="31"/>
+        <nextcloud min-version="27" max-version="31"/>
     </dependencies>
     <settings>
         <admin>OCA\Github\Settings\Admin</admin>

lib/Controller/ConfigController.php

@@ -9,7 +9,10 @@
 use OCA\Github\AppInfo\Application;
 use OCA\Github\Reference\GithubIssuePrReferenceProvider;
 use OCA\Github\Service\GithubAPIService;
+use OCA\Github\Service\SecretService;
 use OCP\AppFramework\Controller;
+use OCP\AppFramework\Http\Attribute\NoAdminRequired;
+use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\Http\RedirectResponse;
 use OCP\AppFramework\Http\TemplateResponse;
@@ -24,27 +27,28 @@
 class ConfigController extends Controller {
 
 	public function __construct(
-		string                                 $appName,
-		IRequest                               $request,
-		private IConfig                        $config,
-		private IURLGenerator                  $urlGenerator,
-		private IL10N                          $l,
-		private IInitialState                  $initialStateService,
-		private GithubAPIService               $githubAPIService,
+		string $appName,
+		IRequest $request,
+		private IConfig $config,
+		private IURLGenerator $urlGenerator,
+		private IL10N $l,
+		private IInitialState $initialStateService,
+		private GithubAPIService $githubAPIService,
+		private SecretService $secretService,
 		private GithubIssuePrReferenceProvider $githubIssuePrReferenceProvider,
-		private ?string                        $userId,
+		private ?string $userId,
 	) {
 		parent::__construct($appName, $request);
 	}
 
 	/**
-	 * @NoAdminRequired
 	 * Set config values
 	 *
 	 * @param array $values key/value pairs to store in user preferences
 	 * @return DataResponse
 	 * @throws PreConditionNotMetException
 	 */
+	#[NoAdminRequired]
 	public function setConfig(array $values): DataResponse {
 		// revoke the oauth token if needed
 		if (isset($values['token']) && $values['token'] === '') {
@@ -56,7 +60,11 @@ public function setConfig(array $values): DataResponse {
 
 		// save values
 		foreach ($values as $key => $value) {
-			$this->config->setUserValue($this->userId, Application::APP_ID, $key, $value);
+			if ($key === 'token') {
+				$this->secretService->setEncryptedUserValue($this->userId, $key, $value);
+			} else {
+				$this->config->setUserValue($this->userId, Application::APP_ID, $key, $value);
+			}
 		}
 		$result = [];
 
@@ -76,6 +84,7 @@ public function setConfig(array $values): DataResponse {
 				$this->config->deleteUserValue($this->userId, Application::APP_ID, 'user_name');
 				$this->config->deleteUserValue($this->userId, Application::APP_ID, 'user_displayname');
 				$this->config->deleteUserValue($this->userId, Application::APP_ID, 'token_type');
+				$this->config->deleteUserValue($this->userId, Application::APP_ID, 'token');
 				$result['user_name'] = '';
 			}
 			// connect or disconnect: invalidate the user-related cache
@@ -92,39 +101,41 @@ public function setConfig(array $values): DataResponse {
 	 */
 	public function setAdminConfig(array $values): DataResponse {
 		foreach ($values as $key => $value) {
-			$this->config->setAppValue(Application::APP_ID, $key, $value);
+			if (in_array($key, ['client_id', 'client_secret', 'default_link_token'], true)) {
+				$this->secretService->setEncryptedAppValue($key, $value);
+			} else {
+				$this->config->setAppValue(Application::APP_ID, $key, $value);
+			}
 		}
 		return new DataResponse(1);
 	}
 
 	/**
-	 * @NoAdminRequired
-	 * @NoCSRFRequired
-	 *
 	 * @param string $user_name
 	 * @param string $user_displayname
 	 * @return TemplateResponse
 	 */
+	#[NoAdminRequired]
+	#[NoCSRFRequired]
 	public function popupSuccessPage(string $user_name, string $user_displayname): TemplateResponse {
 		$this->initialStateService->provideInitialState('popup-data', ['user_name' => $user_name, 'user_displayname' => $user_displayname]);
 		return new TemplateResponse(Application::APP_ID, 'popupSuccess', [], TemplateResponse::RENDER_AS_GUEST);
 	}
 
 	/**
-	 * @NoAdminRequired
-	 * @NoCSRFRequired
-	 *
 	 * Receive oauth code and get oauth access token
 	 *
 	 * @param string $code request code to use when requesting oauth token
 	 * @param string $state value that was sent with original GET request. Used to check auth redirection is valid
 	 * @return RedirectResponse to user settings
 	 * @throws PreConditionNotMetException
 	 */
+	#[NoAdminRequired]
+	#[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');
-		$clientSecret = $this->config->getAppValue(Application::APP_ID, 'client_secret');
+		$clientID = $this->secretService->getEncryptedAppValue('client_id');
+		$clientSecret = $this->secretService->getEncryptedAppValue('client_secret');
 
 		// anyway, reset state
 		$this->config->deleteUserValue($this->userId, Application::APP_ID, 'oauth_state');
@@ -139,7 +150,7 @@ public function oauthRedirect(string $code, string $state): RedirectResponse {
 			if (isset($result['access_token'])) {
 				$this->githubIssuePrReferenceProvider->invalidateUserCache($this->userId);
 				$accessToken = $result['access_token'];
-				$this->config->setUserValue($this->userId, Application::APP_ID, 'token', $accessToken);
+				$this->secretService->setEncryptedUserValue($this->userId, 'token', $accessToken);
 				$this->config->setUserValue($this->userId, Application::APP_ID, 'token_type', 'oauth');
 				$userInfo = $this->storeUserInfo();
 

lib/Controller/GithubAPIController.php

@@ -9,6 +9,8 @@
 use OCA\Github\Service\GithubAPIService;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
+use OCP\AppFramework\Http\Attribute\NoAdminRequired;
+use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
 use OCP\AppFramework\Http\DataDisplayResponse;
 use OCP\AppFramework\Http\DataResponse;
 
@@ -26,13 +28,12 @@ public function __construct(
 	}
 
 	/**
-	 * @NoAdminRequired
-	 *
 	 * Get notification list
 	 *
 	 * @param ?string $since optional date to filter notifications
 	 * @return DataResponse the notifications
 	 */
+	#[NoAdminRequired]
 	public function getNotifications(?string $since = null): DataResponse {
 		$result = $this->githubAPIService->getNotifications($this->userId, false, $since);
 		if (isset($result['error'])) {
@@ -44,12 +45,12 @@ public function getNotifications(?string $since = null): DataResponse {
 	}
 
 	/**
-	 * @NoAdminRequired
-	 *
 	 * Unsubscribe a notification, does the same as in Github notifications page
+	 *
 	 * @param int $id Notification id
 	 * @return DataResponse with request result
 	 */
+	#[NoAdminRequired]
 	public function unsubscribeNotification(int $id): DataResponse {
 		$result = $this->githubAPIService->unsubscribeNotification($this->userId, $id);
 		if (!isset($result['error'])) {
@@ -61,12 +62,12 @@ public function unsubscribeNotification(int $id): DataResponse {
 	}
 
 	/**
-	 * @NoAdminRequired
-	 *
 	 * Mark a notification as read
+	 *
 	 * @param int $id Notification id
 	 * @return DataResponse with request result
 	 */
+	#[NoAdminRequired]
 	public function markNotificationAsRead(int $id): DataResponse {
 		$result = $this->githubAPIService->markNotificationAsRead($this->userId, $id);
 		if (!isset($result['error'])) {
@@ -78,8 +79,6 @@ public function markNotificationAsRead(int $id): DataResponse {
 	}
 
 	/**
-	 * @NoAdminRequired
-	 * @NoCSRFRequired
 	 * TODO remove CSRF and make this endpoint public
 	 * but first: understand why it fails for images and not for data requests
 	 *
@@ -88,6 +87,8 @@ public function markNotificationAsRead(int $id): DataResponse {
 	 * @return DataDisplayResponse The avatar image content
 	 * @throws \Exception
 	 */
+	#[NoAdminRequired]
+	#[NoCSRFRequired]
 	public function getAvatar(string $githubLogin): DataDisplayResponse {
 		$avatar = $this->githubAPIService->getAvatar($this->userId, $githubLogin);
 		if ($avatar !== null && isset($avatar['body'], $avatar['headers'])) {
@@ -103,11 +104,10 @@ public function getAvatar(string $githubLogin): DataDisplayResponse {
 	}
 
 	/**
-	 * @NoAdminRequired
-	 *
 	 * @param string $githubUserLogin
 	 * @return DataResponse
 	 */
+	#[NoAdminRequired]
 	public function getUserInfo(string $githubUserLogin): DataResponse {
 		$result = $this->githubAPIService->getUserInfo($this->userId, $githubUserLogin);
 		if (!isset($result['error'])) {
@@ -120,13 +120,12 @@ public function getUserInfo(string $githubUserLogin): DataResponse {
 	}
 
 	/**
-	 * @NoAdminRequired
-	 *
 	 * @param string $githubUserLogin
 	 * @param string $subjectType
 	 * @param int $subjectId
 	 * @return DataResponse
 	 */
+	#[NoAdminRequired]
 	public function getContextualUserInfo(string $githubUserLogin, string $subjectType, int $subjectId): DataResponse {
 		$result = $this->githubAPIService->getContextualUserInfo($this->userId, $githubUserLogin, $subjectType, $subjectId);
 		if (!isset($result['error'])) {
@@ -139,13 +138,12 @@ public function getContextualUserInfo(string $githubUserLogin, string $subjectTy
 	}
 
 	/**
-	 * @NoAdminRequired
-	 *
 	 * @param string $owner
 	 * @param string $repo
 	 * @param int $issueNumber
 	 * @return DataResponse
 	 */
+	#[NoAdminRequired]
 	public function getIssueInfo(string $owner, string $repo, int $issueNumber): DataResponse {
 		$result = $this->githubAPIService->getIssueInfo($this->userId, $owner, $repo, $issueNumber);
 		if (!isset($result['error'])) {
@@ -156,13 +154,12 @@ public function getIssueInfo(string $owner, string $repo, int $issueNumber): Dat
 	}
 
 	/**
-	 * @NoAdminRequired
-	 *
 	 * @param string $owner
 	 * @param string $repo
 	 * @param int $issueNumber
 	 * @return DataResponse
 	 */
+	#[NoAdminRequired]
 	public function getIssueReactionsInfo(string $owner, string $repo, int $issueNumber): DataResponse {
 		$result = $this->githubAPIService->getIssueReactionsInfo($this->userId, $owner, $repo, $issueNumber);
 		if (!isset($result['error'])) {
@@ -173,13 +170,12 @@ public function getIssueReactionsInfo(string $owner, string $repo, int $issueNum
 	}
 
 	/**
-	 * @NoAdminRequired
-	 *
 	 * @param string $owner
 	 * @param string $repo
 	 * @param int $commentId
 	 * @return DataResponse
 	 */
+	#[NoAdminRequired]
 	public function getIssueCommentInfo(string $owner, string $repo, int $commentId): DataResponse {
 		$result = $this->githubAPIService->getIssueCommentInfo($this->userId, $owner, $repo, $commentId);
 		if (!isset($result['error'])) {
@@ -190,13 +186,12 @@ public function getIssueCommentInfo(string $owner, string $repo, int $commentId)
 	}
 
 	/**
-	 * @NoAdminRequired
-	 *
 	 * @param string $owner
 	 * @param string $repo
 	 * @param int $commentId
 	 * @return DataResponse
 	 */
+	#[NoAdminRequired]
 	public function getIssueCommentReactionsInfo(string $owner, string $repo, int $commentId): DataResponse {
 		$result = $this->githubAPIService->getIssueCommentReactionsInfo($this->userId, $owner, $repo, $commentId);
 		if (!isset($result['error'])) {
@@ -207,13 +202,12 @@ public function getIssueCommentReactionsInfo(string $owner, string $repo, int $c
 	}
 
 	/**
-	 * @NoAdminRequired
-	 *
 	 * @param string $owner
 	 * @param string $repo
 	 * @param int $prNumber
 	 * @return DataResponse
 	 */
+	#[NoAdminRequired]
 	public function getPrInfo(string $owner, string $repo, int $prNumber): DataResponse {
 		$result = $this->githubAPIService->getPrInfo($this->userId, $owner, $repo, $prNumber);
 		if (!isset($result['error'])) {