diff --git a/appinfo/routes.php b/appinfo/routes.php index a73ee012c..0d10500d1 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -41,8 +41,8 @@ ['name' => 'openProjectAPI#getOpenProjectOauthURLWithStateAndPKCE', 'url' => '/op-oauth-url', 'verb' => 'GET'], ['name' => 'openProjectAPI#getProjectFolderSetupStatus', 'url' => '/project-folder-status', 'verb' => 'GET'], ['name' => 'openProjectAPI#getAvailableOpenProjectProjects', 'url' => '/projects','verb' => 'GET'], - ['name' => 'openProjectAPI#getOpenProjectWorkPackageForm', 'url' => '/projects/{id}/work-packages/form','verb' => 'POST'], - ['name' => 'openProjectAPI#getAvailableAssignees', 'url' => '/projects/{id}/available-assignees','verb' => 'GET'], + ['name' => 'openProjectAPI#getOpenProjectWorkPackageForm', 'url' => '/projects/{projectId}/work-packages/form','verb' => 'POST'], + ['name' => 'openProjectAPI#getAvailableAssigneesOfAProject', 'url' => '/projects/{projectId}/available-assignees','verb' => 'GET'], ['name' => 'openProjectAPI#createWorkPackages', 'url' => '/create/work-packages','verb' => 'POST'], ], 'ocs' => [ diff --git a/lib/Controller/OpenProjectAPIController.php b/lib/Controller/OpenProjectAPIController.php index 45a004a0d..bcf7742c7 100644 --- a/lib/Controller/OpenProjectAPIController.php +++ b/lib/Controller/OpenProjectAPIController.php @@ -382,18 +382,24 @@ public function getAvailableOpenProjectProjects(): DataResponse { /** * @NoAdminRequired * - * @param string $id - * @param array $body body is same in the format that openproject api expects the body to be + * @param string $projectId + * @param array|null $body body is same in the format that openproject api expects the body to be * @return DataResponse */ - public function getOpenProjectWorkPackageForm(string $id, array $body): DataResponse { + public function getOpenProjectWorkPackageForm(string $projectId, ?array $body): DataResponse { if ($this->accessToken === '') { return new DataResponse('', Http::STATUS_UNAUTHORIZED); } elseif (!OpenProjectAPIService::validateURL($this->openprojectUrl)) { return new DataResponse('', Http::STATUS_BAD_REQUEST); } + // we don't want to check if all the data in the body is set or not because + // that calculation will be done by the openproject api itself + // we don't want to duplicate the logic + if (empty($body)) { + return new DataResponse(['Body cannot be empty'], Http::STATUS_BAD_REQUEST); + } try { - $result = $this->openprojectAPIService->getOpenProjectWorkPackageForm($this->userId, $id, $body); + $result = $this->openprojectAPIService->getOpenProjectWorkPackageForm($this->userId, $projectId, $body); } catch (OpenprojectErrorException $e) { return new DataResponse($e->getMessage(), $e->getcode()); } catch (\Exception $e) { @@ -403,19 +409,20 @@ public function getOpenProjectWorkPackageForm(string $id, array $body): DataResp } /** + * @NoCSRFRequired * @NoAdminRequired * - * @param string $id + * @param string $projectId * @return DataResponse */ - public function getAvailableAssignees(string $id): DataResponse { + public function getAvailableAssigneesOfAProject(string $projectId): DataResponse { if ($this->accessToken === '') { return new DataResponse('', Http::STATUS_UNAUTHORIZED); } elseif (!OpenProjectAPIService::validateURL($this->openprojectUrl)) { return new DataResponse('', Http::STATUS_BAD_REQUEST); } try { - $result = $this->openprojectAPIService->getAvailableAssignees($this->userId, $id); + $result = $this->openprojectAPIService->getAvailableAssigneesOfAProject($this->userId, $projectId); } catch (OpenprojectErrorException $e) { return new DataResponse($e->getMessage(), $e->getcode()); } catch (\Exception $e) { @@ -425,17 +432,24 @@ public function getAvailableAssignees(string $id): DataResponse { } /** + * @NoCSRFRequired * @NoAdminRequired * - * @param array $body body is same in the format that openproject api expects the body to be + * @param array|null $body body is same in the format that openproject api expects the body to be * @return DataResponse */ - public function createWorkPackages(array $body): DataResponse { + public function createWorkPackages(?array $body): DataResponse { if ($this->accessToken === '') { return new DataResponse('', Http::STATUS_UNAUTHORIZED); } elseif (!OpenProjectAPIService::validateURL($this->openprojectUrl)) { return new DataResponse('', Http::STATUS_BAD_REQUEST); } + // we don't want to check if all the data in the body is set or not because + // that calculation will be done by the openproject api itself + // we don't want to duplicate the logic + if (empty($body)) { + return new DataResponse('Body cannot be empty', Http::STATUS_BAD_REQUEST); + } try { $result = $this->openprojectAPIService->createWorkPackages($this->userId, $body); } catch (OpenprojectErrorException $e) { diff --git a/lib/Listener/AddContentSecurityPolicyListener.php b/lib/Listener/AddContentSecurityPolicyListener.php deleted file mode 100644 index 7164cc111..000000000 --- a/lib/Listener/AddContentSecurityPolicyListener.php +++ /dev/null @@ -1,73 +0,0 @@ - - * - * @author Swikriti Tripathi - * - * @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\Listener; - -use OCA\OpenProject\AppInfo\Application; -use OCP\AppFramework\Http\ContentSecurityPolicy; -use OCP\EventDispatcher\Event; -use OCP\EventDispatcher\IEventListener; -use OCP\IConfig; -use OCP\IRequest; -use OCP\Security\CSP\AddContentSecurityPolicyEvent; - -/** - * @template-implements IEventListener - */ -class AddContentSecurityPolicyListener implements IEventListener { - /** - * @var IConfig - */ - protected $config; - - /** - * @var IRequest - */ - protected $request; - - public function __construct(IConfig $config, IRequest $request) { - $this->config = $config; - $this->request = $request; - } - - public function handle(Event $event): void { - if (!($event instanceof AddContentSecurityPolicyEvent)) { - return; - } - - // only allow through csp if the page is `index.php` i.e files app -// if (!$this->isPageLoad()) { -// return; -// } - - $csp = new ContentSecurityPolicy(); - $baseUrl = $this->config->getAppValue(Application::APP_ID, 'openproject_instance_url', ''); - $csp->addAllowedFrameDomain($baseUrl); - $event->addPolicy($csp); - } - -// private function isPageLoad(): bool { -// $scriptNameParts = explode('/', $this->request->getScriptName()); -// return end($scriptNameParts) === 'index.php'; -// } -} diff --git a/lib/Service/OpenProjectAPIService.php b/lib/Service/OpenProjectAPIService.php index fd602f7de..26736a715 100644 --- a/lib/Service/OpenProjectAPIService.php +++ b/lib/Service/OpenProjectAPIService.php @@ -1342,7 +1342,7 @@ public function getOpenProjectWorkPackageForm(string $userId, string $projectId, * @return array * @throws OpenprojectResponseException|PreConditionNotMetException|OpenprojectErrorException */ - public function getAvailableAssignees(string $userId, string $projectId): array { + public function getAvailableAssigneesOfAProject(string $userId, string $projectId): array { $result = $this->request($userId, 'projects/'.$projectId.'/available_assignees'); if (isset($result['error'])) { throw new OpenprojectErrorException($result['error'], $result['statusCode']); diff --git a/tests/lib/Controller/OpenProjectAPIControllerTest.php b/tests/lib/Controller/OpenProjectAPIControllerTest.php index 8ee020ea5..2004d3962 100644 --- a/tests/lib/Controller/OpenProjectAPIControllerTest.php +++ b/tests/lib/Controller/OpenProjectAPIControllerTest.php @@ -1448,7 +1448,7 @@ public function testGetOpenProjectWorkPackageForm(): void { ], "project" => [ "href" => "/api/v3/projects/1", - "title" > "Demo project" + "title" => "Demo project" ], "assignee" => [ "href" => null @@ -1475,8 +1475,120 @@ public function testGetOpenProjectWorkPackageForm(): void { $this->loggerMock, 'test' ); - $response = $controller->getOpenProjectWorkPackageForm(6, $body); + $response = $controller->getOpenProjectWorkPackageForm('6', $body); $this->assertSame(Http::STATUS_OK, $response->getStatus()); $this->assertSame($result, $response->getData()); } + + /** + * @param \Exception $exception + * @param int $expectedHttpStatusCode + * @param string $expectedError + * @dataProvider exceptionDataProvider + * + *@return void + */ + public function testGetOpenProjectWorkPackageFormException(Exception $exception, int $expectedHttpStatusCode, string $expectedError) { + $this->getUserValueMock(); + $service = $this->getMockBuilder(OpenProjectAPIService::class) + ->disableOriginalConstructor() + ->getMock(); + $service + ->method('getOpenProjectWorkPackageForm') + ->willThrowException($exception); + $controller = new OpenProjectAPIController( + 'integration_openproject', + $this->requestMock, + $this->configMock, + $service, + $this->urlGeneratorMock, + $this->loggerMock, + 'test' + ); + $response = $controller->getOpenProjectWorkPackageForm('6', ["_links" => [ + "type" => [ + "href" => "/api/v3/types/2", + "title" => "Milestone" + ]]]); + $this->assertSame($expectedHttpStatusCode, $response->getStatus()); + $this->assertSame($expectedError, $response->getData()); + } + + public function testGetAvailableAssigneesOfAProject() { + $this->getUserValueMock(); + $result = [ 0 => [ + "_type" => "User", + "id" => 10, + "name" => "openproject admin", + "_links" => [ + "self" => [ + "href" => "/api/v3/users/10", + "title" => "openproject admin" + ] + ] + ], + 1 => [ + "_type" => "User", + "id" => 11, + "name" => "openproject member", + "_links" => [ + "self" => [ + "href" => "/api/v3/users/11", + "title" => "openproject member" + ] + ] + ] + ]; + $service = $this->getMockBuilder(OpenProjectAPIService::class) + ->disableOriginalConstructor() + ->onlyMethods(['getAvailableAssigneesOfAProject']) + ->getMock(); + $service->expects($this->once()) + ->method('getAvailableAssigneesOfAProject') + ->with('test', 6) + ->willReturn($result); + $controller = new OpenProjectAPIController( + 'integration_openproject', + $this->requestMock, + $this->configMock, + $service, + $this->urlGeneratorMock, + $this->loggerMock, + 'test' + ); + $response = $controller->getAvailableAssigneesOfAProject('6'); + $this->assertSame(Http::STATUS_OK, $response->getStatus()); + $this->assertSame( + $result, $response->getData()); + } + + /** + * @param \Exception $exception + * @param int $expectedHttpStatusCode + * @param string $expectedError + * @dataProvider exceptionDataProvider + * + *@return void + */ + public function testGetAvailableAssigneesOfAProjectException(Exception $exception, int $expectedHttpStatusCode, string $expectedError) { + $this->getUserValueMock(); + $service = $this->getMockBuilder(OpenProjectAPIService::class) + ->disableOriginalConstructor() + ->getMock(); + $service + ->method('getAvailableAssigneesOfAProject') + ->willThrowException($exception); + $controller = new OpenProjectAPIController( + 'integration_openproject', + $this->requestMock, + $this->configMock, + $service, + $this->urlGeneratorMock, + $this->loggerMock, + 'test' + ); + $response = $controller->getAvailableAssigneesOfAProject('6'); + $this->assertSame($expectedHttpStatusCode, $response->getStatus()); + $this->assertSame($expectedError, $response->getData()); + } }