fix: use https NC url in OP API requests (#992)

* fix: use https NC url in OP API requests

Signed-off-by: Saw-jan <saw.jan.grg3e@gmail.com>

* chore: add changelog entry

Signed-off-by: Saw-jan <saw.jan.grg3e@gmail.com>

* test: add phpunit test

Signed-off-by: Saw-jan <saw.jan.grg3e@gmail.com>

---------

Signed-off-by: Saw-jan <saw.jan.grg3e@gmail.com>

Author: saw-jan

CHANGELOG.md

@@ -26,6 +26,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ### Fixed
 
 - Fix: Handle projects whose parent project is unknown [#985](https://github.com/nextcloud/integration_openproject/pull/985)
+- Fix: Force HTTPS on Nextcloud base URL for OpenProject API requests [#992](https://github.com/nextcloud/integration_openproject/pull/992)
 
 ### Added
 

lib/Service/OpenProjectAPIService.php

@@ -246,10 +246,25 @@ public function getNotifications(string $userId): array {
 	}
 
 	/**
-	 * wrapper around IURLGenerator::getBaseUrl() to make it easier to mock in tests
+	 * @return string
 	 */
-	public function getBaseUrl(): string {
-		return $this->config->getSystemValueString('overwrite.cli.url');
+	public function getNCBaseUrl(): string {
+		$message = 'Invalid or missing "overwrite.cli.url" system configuration.';
+		$ncUrl = $this->config->getSystemValueString('overwrite.cli.url');
+		if (!$ncUrl) {
+			$this->logger->error($message, ['app' => $this->appName]);
+			return '';
+		}
+
+		$parsedUrl = parse_url($ncUrl);
+		if (!$parsedUrl || !isset($parsedUrl['scheme']) || !isset($parsedUrl['host'])) {
+			$this->logger->error($message, ['app' => $this->appName]);
+			return '';
+		}
+		if ($parsedUrl['scheme'] !== 'https') {
+			$ncUrl = str_replace('http://', 'https://', $ncUrl);
+		}
+		return $ncUrl;
 	}
 
 	/**
@@ -303,7 +318,7 @@ private function searchRequest(string $userId, array $filters, bool $onlyLinkabl
 		if ($onlyLinkableWorkPackages) {
 			$filters[] = [
 				'linkable_to_storage_url' =>
-					['operator' => '=', 'values' => [urlencode($this->getBaseUrl())]]
+					['operator' => '=', 'values' => [urlencode($this->getNCBaseUrl())]]
 			];
 		}
 
@@ -766,7 +781,7 @@ public function linkWorkPackageToFile(
 				],
 				'_links' => [
 					'storageUrl' => [
-						'href' => $this->getBaseUrl()
+						'href' => $this->getNCBaseUrl()
 					]
 				]
 			];
@@ -1520,7 +1535,7 @@ public function getAvailableOpenProjectProjects(string $userId, string $searchQu
 		}
 		$filters[] = [
 			'storageUrl' =>
-				['operator' => '=', 'values' => [$this->getBaseUrl()]],
+				['operator' => '=', 'values' => [$this->getNCBaseUrl()]],
 			'userAction' =>
 				['operator' => '&=', 'values' => ["file_links/manage", "work_packages/create"]]
 		];

tests/lib/Service/OpenProjectAPIServiceTest.php

@@ -803,10 +803,6 @@ private function getOpenProjectAPIService(
 			$appManagerMock->method('isInstalled')->willReturn(true);
 		}
 
-		$urlGeneratorMock = $this->getMockBuilder(IURLGenerator::class)->getMock();
-		$urlGeneratorMock
-			->method('getBaseUrl')
-			->willReturn($baseUrl);
 		$this->defaultConfigMock
 			->method('getSystemValueString')
 			->with($this->equalTo('overwrite.cli.url'))
@@ -817,7 +813,6 @@ private function getOpenProjectAPIService(
 			'config' => $this->defaultConfigMock,
 			'clientService' => $clientService,
 			'rootFolder' => $storageMock,
-			'urlGenerator' => $urlGeneratorMock,
 			'appManager' => $appManagerMock,
 			'tokenEventFactory' => $exchangeTokenMock,
 		]);
@@ -831,21 +826,27 @@ private function getOpenProjectAPIService(
 	 *
 	 * @param array<string> $mockMethods
 	 * @param array<string, object> $constructParams
+	 * @param bool $mockBaseUrl
 	 *
 	 * @return OpenProjectAPIService|MockObject
 	 */
 	private function getOpenProjectAPIServiceMock(
 		array $mockMethods = ['request'],
 		array $constructParams = [],
+		bool $mockBaseUrl = true
 	): OpenProjectAPIService|MockObject {
-		$mockMethods[] = 'getBaseUrl';
+		if ($mockBaseUrl) {
+			$mockMethods[] = 'getNCBaseUrl';
+		}
 		$constructArgs = $this->getOpenProjectAPIServiceConstructArgs($constructParams);
 
 		$mock = $this->getMockBuilder(OpenProjectAPIService::class)
 			->setConstructorArgs($constructArgs)
 			->onlyMethods($mockMethods)
 			->getMock();
-		$mock->method('getBaseUrl')->willReturn('https://nc.my-server.org');
+		if ($mockBaseUrl) {
+			$mock->method('getNCBaseUrl')->willReturn('https://nc.my-server.org');
+		}
 		return $mock;
 	}
 
@@ -3906,14 +3907,7 @@ public function testGetAvailableOpenProjectProjectsErrorResponse(): void {
 	 * @return void
 	 */
 	public function testGetAvailableOpenProjectProjectsQueryOnly() {
-		$iUrlGeneratorMock = $this->getMockBuilder(IURLGenerator::class)->disableOriginalConstructor()->getMock();
-		$iUrlGeneratorMock->method('getBaseUrl')->willReturn('https%3A%2F%2Fnc.my-server.org');
-		$service = $this->getOpenProjectAPIServiceMock(
-			['request'],
-			[
-				'urlGenerator' => $iUrlGeneratorMock,
-			],
-		);
+		$service = $this->getOpenProjectAPIServiceMock(['request']);
 		$service->method('request')
 			->with(
 				'user', 'work_packages/available_projects',
@@ -5197,4 +5191,52 @@ public function testGetAppsName(string $appId, ?array $appInfo, string $expected
 		$this->assertSame($expected, $result);
 	}
 
+	/**
+	 * @return array<mixed>
+	 */
+	public function getNCBaseUrlDataProvider() {
+		return [
+			'HTTP url' => [
+				'url' => 'http://test.nc.local',
+				'expected' => 'https://test.nc.local',
+			],
+			'HTTPS url' => [
+				'url' => 'https://test.nc.local',
+				'expected' => 'https://test.nc.local',
+			],
+			'invalid url' => [
+				'url' => 'invalid_url',
+				'expected' => '',
+			],
+			'empty url' => [
+				'url' => '',
+				'expected' => '',
+			],
+		];
+	}
+
+	/**
+	 * @dataProvider getNCBaseUrlDataProvider
+	 * @param string $url
+	 * @param string $expected
+	 * @return void
+	 */
+	public function testGetNCBaseUrl(string $url, string $expected): void {
+		$configMock = $this->createMock(IConfig::class);
+		$configMock
+			->method('getSystemValueString')
+			->with($this->equalTo('overwrite.cli.url'))
+			->willReturn($url);
+
+		$service = $this->getOpenProjectAPIServiceMock(
+			[],
+			[
+				'config' => $configMock,
+			],
+			false,
+		);
+
+		$baseUrl = $service->getNCBaseUrl();
+		$this->assertEquals($expected, $baseUrl);
+	}
 }