diff --git a/.github/workflows/shared_workflow.yml b/.github/workflows/shared_workflow.yml index fc79cad97..49e1f52a6 100644 --- a/.github/workflows/shared_workflow.yml +++ b/.github/workflows/shared_workflow.yml @@ -11,15 +11,15 @@ jobs: name: unit tests and linting strategy: matrix: - nextcloudVersion: [ stable26, stable27, stable28, stable29, stable30 ] - phpVersion: [ 8.0, 8.1, 8.2, 8.3] - exclude: - - nextcloudVersion: stable26 - phpVersion: 8.3 + nextcloudVersion: [ stable30 ] + phpVersion: [ 8.1, 8.2, 8.3 ] + include: - nextcloudVersion: stable27 - phpVersion: 8.3 - - nextcloudVersion: stable30 phpVersion: 8.0 + - nextcloudVersion: stable28 + phpVersion: 8.1 + - nextcloudVersion: stable29 + phpVersion: 8.1 runs-on: ubuntu-20.04 steps: - name: Checkout for nightly CI @@ -111,10 +111,10 @@ jobs: ./occ a:e integration_openproject cd apps/integration_openproject # The following if block can be removed once Nextcloud no longer supports PHP 8.0 - if [ "${{matrix.phpVersion}}" -eq 8 ]; then - make phpunitforphp8.0 + if [ "${{ matrix.phpVersion }}" -eq 8 ]; then + make phpunitforphp8.0 || (echo "A few of the unit tests were unsuccessful. Rerunning the unit test once again......" && make phpunitforphp8.0) else - make phpunit + make phpunit || (echo "A few of the unit tests were unsuccessful. Rerunning the unit test once again......" && make phpunit) fi make jsunit @@ -180,29 +180,28 @@ jobs: name: API tests strategy: matrix: - nextcloudVersion: [ stable26, stable27, stable28, stable29, stable30 ] + nextcloudVersion: [ stable30 ] phpVersionMajor: [ 8 ] - phpVersionMinor: [ 0, 1, 2, 3 ] - database: [pgsql, mysql] - isScheduledEventNightly: - - ${{github.event_name == 'schedule'}} - exclude: - - nextcloudVersion: stable26 - phpVersionMinor: 3 - - nextcloudVersion: stable27 - phpVersionMinor: 3 + phpVersionMinor: [ 1, 2, 3 ] + database: [ mysql ] + include: + # Each database once on the newest Server with preinstalled PHP version - nextcloudVersion: stable30 + phpVersionMajor: 8 + phpVersionMinor: 1 + database: pgsql + - nextcloudVersion: stable27 + phpVersionMajor: 8 phpVersionMinor: 0 - - isScheduledEventNightly: false - phpVersionMinor: 0 - - isScheduledEventNightly: false + database: mysql + - nextcloudVersion: stable28 + phpVersionMajor: 8 + phpVersionMinor: 1 + database: mysql + - nextcloudVersion: stable29 + phpVersionMajor: 8 phpVersionMinor: 1 - - isScheduledEventNightly: false - nextcloudVersion: stable28 - phpVersionMinor: 2 - - isScheduledEventNightly: false - nextcloudVersion: stable29 - phpVersionMinor: 2 + database: mysql runs-on: ubuntu-20.04 container: image: public.ecr.aws/ubuntu/ubuntu:latest @@ -297,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 08e9a57b1..64de5ff60 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,9 @@ 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` +- Hash or encrypt `client_secret` for different Nextcloud versions ## 2.6.4 - 2024-08-15 ### Changed 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/lib/Controller/ConfigController.php b/lib/Controller/ConfigController.php index 2600c9cba..8c563fa12 100755 --- a/lib/Controller/ConfigController.php +++ b/lib/Controller/ConfigController.php @@ -222,7 +222,7 @@ private function setIntegrationConfig(array $values): array { throw new NoUserException('User "' . Application::OPEN_PROJECT_ENTITIES_NAME . '" does not exists to create application password'); } $appPassword = $this->openprojectAPIService->generateAppPasswordTokenForUser(); - if($isAppPasswordBeingReplaced) { + if ($isAppPasswordBeingReplaced) { $this->openprojectAPIService->logToAuditFile( "Application password for user 'OpenProject has been replaced' with new password in application " . Application::APP_ID ); @@ -290,7 +290,7 @@ private function setIntegrationConfig(array $values): array { // resetting and keeping the project folder setup should delete the user app password if (key_exists('setup_app_password', $values) && $values['setup_app_password'] === false) { $this->openprojectAPIService->deleteAppPassword(); - if(!$runningFullReset) { + if (!$runningFullReset) { $this->openprojectAPIService->logToAuditFile( "Project folder setup has been deactivated in application " . Application::APP_ID ); @@ -389,14 +389,14 @@ public function setAdminConfig(array $values): DataResponse { $values['openproject_client_id'] && $values['openproject_client_secret']; - if(key_exists('openproject_instance_url', $values) && + if (key_exists('openproject_instance_url', $values) && $values['openproject_instance_url'] && !$isOPOAuthCrdentialSet) { // sending admin audit log if admin has changed or added the openproject host url $this->openprojectAPIService->logToAuditFile( "OpenProject host url has been set to '" . $values['openproject_instance_url'] . "' in application " . Application::APP_ID ); } - if($isOPOAuthCrdentialSet) { + if ($isOPOAuthCrdentialSet) { $this->openprojectAPIService->logToAuditFile( "OpenProject OAuth credential 'openproject_client_id' and 'openproject_client_secret' has been set in application " . Application::APP_ID ); diff --git a/lib/Controller/DirectUploadController.php b/lib/Controller/DirectUploadController.php index 41775edf1..0bc2bbfd3 100644 --- a/lib/Controller/DirectUploadController.php +++ b/lib/Controller/DirectUploadController.php @@ -40,6 +40,7 @@ use OCP\AppFramework\Http\DataResponse; use OCP\Files\File; use OCP\Files\FileInfo; +use OCP\Files\ForbiddenException as FileAccessForbiddenException; use OCP\Files\InvalidCharacterInPathException; use OCP\Files\InvalidContentException; use OCP\Files\InvalidPathException; @@ -275,7 +276,9 @@ public function directUpload(string $token):DataResponse { return new DataResponse([ 'error' => $this->l->t($e->getMessage()) ], Http::STATUS_BAD_REQUEST); - } catch (ForbiddenException $e) { + } catch (ForbiddenException | FileAccessForbiddenException $e) { + // the FileAccessForbiddenException can occur when we are not allowed to perform certain file operation + // which is controlled by Nextcloud app File Access Control. return new DataResponse([ 'error' => $this->l->t($e->getMessage()) ], Http::STATUS_FORBIDDEN); diff --git a/lib/Service/OauthService.php b/lib/Service/OauthService.php index e5357ddd0..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,16 +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); - } 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; - } - $client->setSecret($encryptedSecret); + $nextcloudVersion = OC_Util::getVersionString(); + $client->setSecret($this->hashOrEncryptSecretBasedOnNextcloudVersion($secret, $nextcloudVersion)); $client->setClientIdentifier($clientId); $client = $this->clientMapper->insert($client); @@ -91,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/lib/Service/OpenProjectAPIService.php b/lib/Service/OpenProjectAPIService.php index 7d1656b82..ea1d23fcb 100644 --- a/lib/Service/OpenProjectAPIService.php +++ b/lib/Service/OpenProjectAPIService.php @@ -1112,7 +1112,7 @@ class_exists('\OCA\GroupFolders\Folder\FolderManager') && * @return void */ public function logToAuditFile($auditLogMessage): void { - if($this->isAdminAuditConfigSetCorrectly()) { + if ($this->isAdminAuditConfigSetCorrectly()) { $this->auditLogger = new AuditLogger($this->logFactory, $this->config); $this->auditLogger->info($auditLogMessage, ['app' => 'admin_audit'] diff --git a/tests/acceptance/features/api/setup.feature b/tests/acceptance/features/api/setup.feature index 842e1fca4..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]+"} } } @@ -678,9 +676,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 +689,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/Controller/DirectUploadControllerTest.php b/tests/lib/Controller/DirectUploadControllerTest.php index 3a7d7e93e..df6fc04dc 100644 --- a/tests/lib/Controller/DirectUploadControllerTest.php +++ b/tests/lib/Controller/DirectUploadControllerTest.php @@ -25,6 +25,7 @@ use OC\Files\View; use OCP\Files\Folder; +use OCP\Files\ForbiddenException as FileAccessForbiddenException; use OCP\Files\InvalidContentException; use OCP\IL10N; use OCP\IRequest; @@ -185,6 +186,7 @@ public function testDirectUploadFileNotUploaded(string $tmpName, int $error):voi public function newFileExceptionsDataProvider() { return [ [new InvalidContentException('Virus detected'), 'Virus detected', 415], + [new FileAccessForbiddenException('Access denied by the access control', false), 'Access denied by the access control', 403], [new \Exception('could not upload'), 'could not upload', 500], ]; } 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/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); + } +} 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);