[WP#57654]switch encryption for oauth secret

CHANGELOG.md

@@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Enhance project search when creating workpackages from Nextcloud
 - Drop application's support for Nextcloud 26
 - Fix issue preventing direct uploading of resources in Nextcloud that are managed by app `Files Access Control`
+- Hash or encrypt `client_secret` for different Nextcloud versions
 
 ## 2.6.4 - 2024-08-15
 ### Changed

lib/Service/OauthService.php

@@ -47,6 +47,29 @@ public function __construct(ClientMapper $clientMapper,
 		$this->crypto = $crypto;
 	}
 
+	/**
+	 * @param string $secret
+	 * @param string $nextcloudVersion
+	 * @return string
+	 */
+	public function getHashedOrEncryptedSecretBasedOnNextcloudVersions(string $secret, string $nextcloudVersion): string {
+		switch (true) {
+			case version_compare($nextcloudVersion, '30.0.0') >= 0:
+			case version_compare($nextcloudVersion, '29.0.7') >= 0 && version_compare($nextcloudVersion, '30.0.0') < 0:
+			case version_compare($nextcloudVersion, '28.0.10') >= 0 && version_compare($nextcloudVersion, '29.0.0') < 0:
+			case version_compare($nextcloudVersion, '27.1.11.8') >= 0 && version_compare($nextcloudVersion, '28.0.0') < 0:
+				$encryptedSecret = bin2hex($this->crypto->calculateHMAC($secret));
+				break;
+			case version_compare($nextcloudVersion, '27.0.0') === 0:
+				$encryptedSecret = $secret;
+				break;
+			default:
+				$encryptedSecret = $this->crypto->encrypt($secret);
+				break;
+		}
+		return $encryptedSecret;
+	}
+
 	/**
 	 * @param string $name
 	 * @param string $redirectUri
@@ -58,12 +81,8 @@ public function createNcOauthClient(string $name, string $redirectUri): array {
 		$client->setName($name);
 		$client->setRedirectUri(sprintf($redirectUri, $clientId));
 		$secret = $this->secureRandom->generate(64, self::validChars);
-		if (version_compare(OC_Util::getVersionString(), '27.0.1') >= 0) {
-			$encryptedSecret = $this->crypto->encrypt($secret);
-		} else {
-			$encryptedSecret = $secret;
-		}
-		$client->setSecret($encryptedSecret);
+		$nextcloudVersion = OC_Util::getVersionString();
+		$client->setSecret($this->getHashedOrEncryptedSecretBasedOnNextcloudVersions($secret, $nextcloudVersion));
 		$client->setClientIdentifier($clientId);
 		$client = $this->clientMapper->insert($client);
 
@@ -87,8 +106,7 @@ public function getClientInfo(int $id): ?array {
 				'id' => $client->getId(),
 				'nextcloud_oauth_client_name' => $client->getName(),
 				'openproject_redirect_uri' => $client->getRedirectUri(),
-				'nextcloud_client_id' => $client->getClientIdentifier(),
-				'nextcloud_client_secret' => $this->crypto->decrypt($client->getSecret()),
+				'nextcloud_client_id' => $client->getClientIdentifier()
 			];
 		} catch (ClientNotFoundException $e) {
 			return null;

tests/acceptance/features/api/setup.feature

@@ -656,14 +656,12 @@ Feature: setup the integration through an API
       "required": [
           "nextcloud_oauth_client_name",
           "openproject_redirect_uri",
-          "nextcloud_client_id",
-          "nextcloud_client_secret"
+          "nextcloud_client_id"
        ],
       "properties": {
           "nextcloud_oauth_client_name": {"type": "string", "pattern": "^OpenProject client$"},
           "openproject_redirect_uri": {"type": "string", "pattern": "^http:\/\/some-host.de\/oauth_clients\/[A-Za-z0-9]+\/callback$"},
           "nextcloud_client_id": {"type": "string", "pattern": "[A-Za-z0-9]+"},
-          "nextcloud_client_secret": {"type": "string", "pattern": "[A-Za-z0-9]+"},
           "openproject_user_app_password": {"type": "string", "pattern": "[A-Za-z0-9]+"}
       }
     }

tests/lib/Service/OauthSeviceTest.php

@@ -0,0 +1,123 @@
+<?php
+/**
+ * @copyright Copyright (c) 2022 Swikriti Tripathi <[email protected]>
+ *
+ * @author Your name <[email protected]>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OCA\OpenProject\Service;
+
+use OCA\OAuth2\Db\ClientMapper;
+use OCP\Security\ICrypto;
+use OCP\Security\ISecureRandom;
+use PHPUnit\Framework\TestCase;
+
+class OauthServiceTest extends TestCase {
+	protected function getOauthServiceMock(
+		$clientMapperMock = null,
+		$iSecureRandomMock = null,
+		$iCryptoMock = null,
+	): OauthService {
+
+		if ($clientMapperMock === null) {
+			$clientMapperMock = $this->getMockBuilder(ClientMapper::class)->disableOriginalConstructor()->getMock();
+		}
+		if ($iSecureRandomMock === null) {
+			$iSecureRandomMock = $this->getMockBuilder(ISecureRandom::class)->getMock();
+		}
+		if ($iCryptoMock === null) {
+			$iCryptoMock = $this->getMockBuilder(ICrypto::class)->getMock();
+		}
+
+		return new OauthService(
+			$clientMapperMock,
+			$iSecureRandomMock,
+			$iCryptoMock
+		);
+	}
+
+
+	/**
+	 * @return array<mixed>
+	 */
+	public function getHashedOrEncryptedSecretBasedOnNextcloudVersionsDataProvider() {
+		return [
+			[
+				"30.0.0",
+				"calculateHMAC"
+			],
+			[
+				"29.0.7",
+				"calculateHMAC"
+			],
+			[
+				"29.1.0",
+				"calculateHMAC"
+			],
+			[
+				"29.0.6",
+				"encrypt"
+			],
+			[
+				"28.0.10",
+				"calculateHMAC"
+			],
+			[
+				"28.2.0",
+				"calculateHMAC"
+			],
+			[
+				"28.0.0",
+				"encrypt"
+			],
+			[
+				"29.0.0",
+				"encrypt"
+			],
+			[
+				"27.1.11.8",
+				"calculateHMAC"
+			],
+			[
+				"27.1.12.0",
+				"calculateHMAC"
+			],
+			[
+				"27.1.1.0",
+				"encrypt"
+			]
+		];
+	}
+
+
+	/**
+	 * @dataProvider getHashedOrEncryptedSecretBasedOnNextcloudVersionsDataProvider
+	 * @param string $nextcloudVersion
+	 * @param string $hashOrEncryptFunction
+	 *
+	 * @return void
+	 *
+	 */
+	public function testGetHashedOrEncryptedClientSecretBasedOnNextcloudVersions(string $nextcloudVersion, string $hashOrEncryptFunction) {
+		$iCryptoMock = $this->getMockBuilder(ICrypto::class)->getMock();
+		$oAuthService = $this->getOauthServiceMock(null, null, $iCryptoMock);
+		$iCryptoMock->expects($this->once())->method($hashOrEncryptFunction);
+		$oAuthService->getHashedOrEncryptedSecretBasedOnNextcloudVersions("client_secret", $nextcloudVersion);
+	}
+}