From 0b3c9cd5f1b18d0ab75415f52fc82eb68f195c04 Mon Sep 17 00:00:00 2001 From: Sagar Gurung <46086950+SagarGi@users.noreply.github.com> Date: Mon, 26 Aug 2024 09:20:04 +0545 Subject: [PATCH 1/3] Drop support for nc 26 (#689) * Adjust or remove code specific to NC-26 Signed-off-by: Sagar * Adjust or remove things realted to stable 26 Signed-off-by: Sagar * fix CI failure Signed-off-by: Sagar * update changelog Signed-off-by: Sagar --------- Signed-off-by: Sagar --- .github/workflows/shared_workflow.yml | 11 --- CHANGELOG.md | 1 + appinfo/info.xml | 2 +- tests/acceptance/features/api/setup.feature | 8 +- .../WorkPackageReferenceProviderTest.php | 7 -- .../lib/Service/OpenProjectAPIServiceTest.php | 75 +++++++------------ 6 files changed, 33 insertions(+), 71 deletions(-) diff --git a/.github/workflows/shared_workflow.yml b/.github/workflows/shared_workflow.yml index 9facfd4f1..49e1f52a6 100644 --- a/.github/workflows/shared_workflow.yml +++ b/.github/workflows/shared_workflow.yml @@ -14,8 +14,6 @@ jobs: nextcloudVersion: [ stable30 ] phpVersion: [ 8.1, 8.2, 8.3 ] include: - - nextcloudVersion: stable26 - phpVersion: 8.0 - nextcloudVersion: stable27 phpVersion: 8.0 - nextcloudVersion: stable28 @@ -192,11 +190,6 @@ jobs: phpVersionMajor: 8 phpVersionMinor: 1 database: pgsql - # Each oldServer with the oldest PHP version and one database - - nextcloudVersion: stable26 - phpVersionMajor: 8 - phpVersionMinor: 0 - database: mysql - nextcloudVersion: stable27 phpVersionMajor: 8 phpVersionMinor: 0 @@ -303,10 +296,6 @@ jobs: - name: API Tests env: NEXTCLOUD_BASE_URL: http://nextcloud - BEHAT_FILTER_TAGS: ${{ - matrix.nextcloudVersion == 'stable26' && '~@skipOnStable26' || - '' - }} run: | # The following if block can be removed once Nextcloud no longer supports PHP 8.0 if [ "${{matrix.phpVersionMajor}}" -eq 8 ] && [ "${{matrix.phpVersionMinor}}" -eq 0 ]; then diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ed5750a4..74e002231 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Fix random deactivation of automatically managed project folder - Fix avatar not found in openproject - 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` ## 2.6.4 - 2024-08-15 diff --git a/appinfo/info.xml b/appinfo/info.xml index 5b700bc32..6afe609ab 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -41,7 +41,7 @@ For more information on how to set up and use the OpenProject application, pleas https://github.com/nextcloud/integration_openproject/raw/master/img/screenshot1.png https://github.com/nextcloud/integration_openproject/raw/master/img/screenshot2.png - + OCA\OpenProject\BackgroundJob\RemoveExpiredDirectUploadTokens diff --git a/tests/acceptance/features/api/setup.feature b/tests/acceptance/features/api/setup.feature index 842e1fca4..c7d01bb7e 100644 --- a/tests/acceptance/features/api/setup.feature +++ b/tests/acceptance/features/api/setup.feature @@ -678,9 +678,7 @@ Feature: setup the integration through an API When user "OpenProject" sends a "PROPFIND" request to "/remote.php/webdav" using old app password Then the HTTP status code should be "401" - # to locally run this test the "project folder" needs to be setup already - # issue of group folder https://github.com/nextcloud/groupfolders/issues/2718 - @skipOnStable25 @skipOnStable26 + Scenario: check version of uploaded file inside a group folder Given user "Carol" has been created And user "Carol" has been added to the group "OpenProject" @@ -693,9 +691,7 @@ Feature: setup the integration through an API When user "Carol" deletes folder "/OpenProject/OpenProject/project-demo" Then the HTTP status code should be 204 - # to locally run this test the "project folder" needs to be setup already - # issue of group folder https://github.com/nextcloud/groupfolders/issues/2718 - @skipOnStable25 @skipOnStable26 + Scenario: check version of uploaded file after an update inside a group folder Given user "Carol" has been created And user "Carol" has been added to the group "OpenProject" diff --git a/tests/lib/Reference/WorkPackageReferenceProviderTest.php b/tests/lib/Reference/WorkPackageReferenceProviderTest.php index b17ff704a..c7cfa16d3 100644 --- a/tests/lib/Reference/WorkPackageReferenceProviderTest.php +++ b/tests/lib/Reference/WorkPackageReferenceProviderTest.php @@ -24,7 +24,6 @@ namespace OCA\OpenProject\Reference; use OC\Collaboration\Reference\ReferenceManager; -use OC_Util; use OCA\OpenProject\AppInfo\Application; use OCA\OpenProject\Service\OpenProjectAPIService; use OCP\IConfig; @@ -34,12 +33,6 @@ use PHPUnit\Framework\TestCase; class WorkPackageReferenceProviderTest extends TestCase { - protected function setUp(): void { - if (version_compare(OC_Util::getVersionString(), '26') < 0) { - $this->markTestSkipped('WorkPackageReferenceProvider is only available from nextcloud 26 so skip the tests on versions below'); - } - } - /** * * @param array $onlyMethods diff --git a/tests/lib/Service/OpenProjectAPIServiceTest.php b/tests/lib/Service/OpenProjectAPIServiceTest.php index 68ce11d77..50f0d5906 100644 --- a/tests/lib/Service/OpenProjectAPIServiceTest.php +++ b/tests/lib/Service/OpenProjectAPIServiceTest.php @@ -19,7 +19,6 @@ use OC\Authentication\Token\IToken; use OC\Avatar\GuestAvatar; use OC\Http\Client\Client; -use OC_Util; use OCA\GroupFolders\Folder\FolderManager; use OCA\OpenProject\AppInfo\Application; use OCA\OpenProject\Exception\OpenprojectErrorException; @@ -639,53 +638,37 @@ private function getOpenProjectAPIService( $ocClient = null; $client = new GuzzleClient(); $clientConfigMock = $this->getMockBuilder(IConfig::class)->getMock(); - - if (version_compare(OC_Util::getVersionString(), '27') >= 0) { - $clientConfigMock - ->method('getSystemValueBool') - ->withConsecutive( - ['allow_local_remote_servers', false], - ['installed', false], - ['allow_local_remote_servers', false], - ['allow_local_remote_servers', false], - ['installed', false], - ['allow_local_remote_servers', false], - ['allow_local_remote_servers', false], - ['installed', false], - ['allow_local_remote_servers', false] - ) - ->willReturnOnConsecutiveCalls( - true, - true, - true, - true, - true, - true, - true, - true, - true - ); - //changed from nextcloud 26 - $ocClient = new Client( - $clientConfigMock, - $certificateManager, - $client, - $this->createMock(IRemoteHostValidator::class), - $this->createMock(LoggerInterface::class)); - } elseif (version_compare(OC_Util::getVersionString(), '26') >= 0) { - $clientConfigMock + $clientConfigMock ->method('getSystemValueBool') - ->with('allow_local_remote_servers', false) - ->willReturn(true); - - //changed from nextcloud 26 - $ocClient = new Client( - $clientConfigMock, - $certificateManager, - $client, - $this->createMock(IRemoteHostValidator::class) + ->withConsecutive( + ['allow_local_remote_servers', false], + ['installed', false], + ['allow_local_remote_servers', false], + ['allow_local_remote_servers', false], + ['installed', false], + ['allow_local_remote_servers', false], + ['allow_local_remote_servers', false], + ['installed', false], + ['allow_local_remote_servers', false] + ) + ->willReturnOnConsecutiveCalls( + true, + true, + true, + true, + true, + true, + true, + true, + true ); - } + //changed from nextcloud 26 + $ocClient = new Client( + $clientConfigMock, + $certificateManager, + $client, + $this->createMock(IRemoteHostValidator::class), + $this->createMock(LoggerInterface::class)); $clientService = $this->getMockBuilder('\OCP\Http\Client\IClientService')->getMock(); $clientService->method('newClient')->willReturn($ocClient); From 0d5a02b392d2abe0c45b76fc310895eafa96648f Mon Sep 17 00:00:00 2001 From: Sagar Gurung <46086950+SagarGi@users.noreply.github.com> Date: Thu, 5 Sep 2024 15:43:11 +0545 Subject: [PATCH 2/3] Remove version comparision for secret encryption (#695) * Remove version comparision for secret encryption Signed-off-by: Sagar * stlye fix Signed-off-by: Sagar --------- Signed-off-by: Sagar --- lib/Service/OauthService.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/Service/OauthService.php b/lib/Service/OauthService.php index e5357ddd0..547f64a4d 100644 --- a/lib/Service/OauthService.php +++ b/lib/Service/OauthService.php @@ -60,10 +60,6 @@ public function createNcOauthClient(string $name, string $redirectUri): array { $secret = $this->secureRandom->generate(64, self::validChars); if (version_compare(OC_Util::getVersionString(), '27.0.1') >= 0) { $encryptedSecret = $this->crypto->encrypt($secret); - } elseif (version_compare(OC_Util::getVersionString(), '26.0.4') >= 0 && version_compare(OC_Util::getVersionString(), '27.0.0') < 0) { - $encryptedSecret = $this->crypto->encrypt($secret); - } elseif (version_compare(OC_Util::getVersionString(), '25.0.8') >= 0 && version_compare(OC_Util::getVersionString(), '26.0.0') < 0) { - $encryptedSecret = $this->crypto->encrypt($secret); } else { $encryptedSecret = $secret; } From b49283d97f62755ca753588a84d1a0d5e3284801 Mon Sep 17 00:00:00 2001 From: Sagar Gurung <46086950+SagarGi@users.noreply.github.com> Date: Fri, 6 Sep 2024 17:11:55 +0545 Subject: [PATCH 3/3] [WP#57654]switch encryption for oauth secret (#694) --- CHANGELOG.md | 1 + lib/Service/OauthService.php | 34 ++++-- tests/acceptance/features/api/setup.feature | 4 +- tests/lib/Service/OauthSeviceTest.php | 123 ++++++++++++++++++++ 4 files changed, 151 insertions(+), 11 deletions(-) create mode 100644 tests/lib/Service/OauthSeviceTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 74e002231..64de5ff60 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/lib/Service/OauthService.php b/lib/Service/OauthService.php index 547f64a4d..21c25000e 100644 --- a/lib/Service/OauthService.php +++ b/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 hashOrEncryptSecretBasedOnNextcloudVersion(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->hashOrEncryptSecretBasedOnNextcloudVersion($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; diff --git a/tests/acceptance/features/api/setup.feature b/tests/acceptance/features/api/setup.feature index c7d01bb7e..40f165560 100644 --- a/tests/acceptance/features/api/setup.feature +++ b/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]+"} } } diff --git a/tests/lib/Service/OauthSeviceTest.php b/tests/lib/Service/OauthSeviceTest.php new file mode 100644 index 000000000..e49242989 --- /dev/null +++ b/tests/lib/Service/OauthSeviceTest.php @@ -0,0 +1,123 @@ + + * + * @author Your name + * + * @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 . + * + */ + +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 + */ + public function gethashOrEncryptSecretBasedOnNextcloudVersion(): array { + 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 gethashOrEncryptSecretBasedOnNextcloudVersion + * @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->hashOrEncryptSecretBasedOnNextcloudVersion("client_secret", $nextcloudVersion); + } +}