ExApp proxy adjustments and fixes

appinfo/routes.php

@@ -15,7 +15,7 @@
 		// Proxy
 		['name' => 'ExAppProxy#ExAppGet',
 			'url' => '/proxy/{appId}/{other}', 'verb' => 'GET' , 'root' => '/proxy',
-			'requirements' => ['other' => '.+'], 'defaults' => ['other' => '']],
+			'requirements' => ['other' => '.*'], 'defaults' => ['other' => '']],
 		['name' => 'ExAppProxy#ExAppPost',
 			'url' => '/proxy/{appId}/{other}', 'verb' => 'POST' , 'root' => '/proxy',
 			'requirements' => ['other' => '.+'], 'defaults' => ['other' => '']],

lib/Controller/ExAppProxyController.php

@@ -4,6 +4,9 @@
 
 namespace OCA\AppAPI\Controller;
 
+use GuzzleHttp\Cookie\CookieJar;
+use GuzzleHttp\Cookie\SetCookie;
+use GuzzleHttp\RequestOptions;
 use OC\Security\CSP\ContentSecurityPolicyNonceManager;
 use OCA\AppAPI\AppInfo\Application;
 use OCA\AppAPI\ProxyResponse;
@@ -62,7 +65,9 @@ private function createProxyResponse(string $path, IResponse $response, $cache =
 		}
 
 		$proxyResponse = new ProxyResponse($response->getStatusCode(), $responseHeaders, $content);
-		if ($cache && !$isHTML && empty($response->getHeader('cache-control'))) {
+		if ($cache && !$isHTML && empty($response->getHeader('cache-control'))
+			&& $response->getHeader('Content-Type') !== 'application/json'
+			&& $response->getHeader('Content-Type') !== 'application/x-tar') {
 			$proxyResponse->cacheFor(3600);
 		}
 		return $proxyResponse;
@@ -76,8 +81,11 @@ public function ExAppGet(string $appId, string $other): Response {
 			return new NotFoundResponse();
 		}
 
-		$response = $this->service->requestToExApp(
-			$exApp, '/' . $other, $this->userId, 'GET', request: $this->request
+		$response = $this->service->aeRequestToExApp2(
+			$exApp, '/' . $other, $this->userId, 'GET', queryParams: $_GET, options: [
+				RequestOptions::COOKIES => $this->buildProxyCookiesJar($_COOKIE, $exApp->getHost()),
+			],
+			request: $this->request,
 		);
 		if (is_array($response)) {
 			return (new Response())->setStatus(500);
@@ -93,9 +101,19 @@ public function ExAppPost(string $appId, string $other): Response {
 			return new NotFoundResponse();
 		}
 
-		$response = $this->service->aeRequestToExApp(
+		$options = [
+			RequestOptions::COOKIES => $this->buildProxyCookiesJar($_COOKIE, $exApp->getHost()),
+		];
+		if (str_starts_with($this->request->getHeader('Content-Type'), 'multipart/form-data') || count($_FILES) > 0) {
+			$multipart = $this->buildMultipartFormData($this->prepareBodyParams($this->request->getParams()), $_FILES);
+			$options[RequestOptions::MULTIPART] = $multipart;
+		}
+		$bodyParams = $this->prepareBodyParams($this->request->getParams());
+
+		$response = $this->service->aeRequestToExApp2(
 			$exApp, '/' . $other, $this->userId,
-			params: $this->request->getParams(), request: $this->request
+			queryParams: $_GET, bodyParams: $bodyParams, options: $options,
+			request: $this->request,
 		);
 		if (is_array($response)) {
 			return (new Response())->setStatus(500);
@@ -111,8 +129,19 @@ public function ExAppPut(string $appId, string $other): Response {
 			return new NotFoundResponse();
 		}
 
-		$response = $this->service->aeRequestToExApp(
-			$exApp, '/' . $other, $this->userId, 'PUT', $this->request->getParams(), request: $this->request
+		$options = [
+			RequestOptions::COOKIES => $this->buildProxyCookiesJar($_COOKIE, $exApp->getHost()),
+		];
+		if (str_starts_with($this->request->getHeader('Content-Type'), 'multipart/form-data') || count($_FILES) > 0) {
+			$multipart = $this->buildMultipartFormData($this->prepareBodyParams($this->request->getParams()), $_FILES);
+			$options[RequestOptions::MULTIPART] = $multipart;
+		}
+		$bodyParams = $this->prepareBodyParams($this->request->getParams());
+
+		$response = $this->service->aeRequestToExApp2(
+			$exApp, '/' . $other, $this->userId, 'PUT', queryParams: $_GET, bodyParams: $bodyParams,
+			options: $options,
+			request: $this->request,
 		);
 		if (is_array($response)) {
 			return (new Response())->setStatus(500);
@@ -128,12 +157,61 @@ public function ExAppDelete(string $appId, string $other): Response {
 			return new NotFoundResponse();
 		}
 
-		$response = $this->service->aeRequestToExApp(
-			$exApp, '/' . $other, $this->userId, 'DELETE', $this->request->getParams(), request: $this->request
+		$bodyParams = $this->prepareBodyParams($this->request->getParams());
+		$response = $this->service->aeRequestToExApp2(
+			$exApp, '/' . $other, $this->userId, 'DELETE', queryParams: $_GET, bodyParams: $bodyParams,
+			options: [
+				RequestOptions::COOKIES => $this->buildProxyCookiesJar($_COOKIE, $exApp->getHost()),
+			],
+			request: $this->request,
 		);
 		if (is_array($response)) {
 			return (new Response())->setStatus(500);
 		}
 		return $this->createProxyResponse($other, $response);
 	}
+
+	private function buildProxyCookiesJar(array $cookies, string $domain): CookieJar {
+		$cookieJar = new CookieJar();
+		foreach ($cookies as $name => $value) {
+			$cookieJar->setCookie(new SetCookie([
+				'Domain' => $domain,
+				'Name' => $name,
+				'Value' => $value,
+				'Discard' => true,
+				'Secure' => false,
+				'HttpOnly' => true,
+			]));
+		}
+		return $cookieJar;
+	}
+
+	private function prepareBodyParams(array $params): array {
+		unset($params['appId'], $params['other'], $params['_route']);
+		foreach ($_GET as $key => $value) {
+			unset($params[$key]);
+		}
+		return $params;
+	}
+
+	/**
+	 * Build the multipart form data from input parameters and files
+	 */
+	private function buildMultipartFormData(array $bodyParams, array $files): array {
+		$multipart = [];
+		foreach ($bodyParams as $key => $value) {
+			$multipart[] = [
+				'name' => $key,
+				'contents' => $value,
+			];
+		}
+		foreach ($files as $key => $file) {
+			$multipart[] = [
+				'name' => $key,
+				'contents' => fopen($file['tmp_name'], 'r'),
+				'filename' => $file['name'],
+			];
+		}
+		return $multipart;
+	}
 }

lib/Service/AppAPIService.php

@@ -73,6 +73,29 @@ public function aeRequestToExApp(
 		return $this->requestToExApp($exApp, $route, $userId, $method, $params, $options, $request);
 	}
 
+	/**
+	 * Request to ExApp with AppAPI auth headers and ExApp user initialization
+	 * with more correct query/body params handling
+	 */
+	public function aeRequestToExApp2(
+		ExApp $exApp,
+		string $route,
+		?string $userId = null,
+		string $method = 'POST',
+		array $queryParams = [],
+		array $bodyParams = [],
+		array $options = [],
+		?IRequest $request = null,
+	): array|IResponse {
+		try {
+			$this->exAppUsersService->setupExAppUser($exApp->getAppid(), $userId);
+		} catch (\Exception $e) {
+			$this->logger->error(sprintf('Error while inserting ExApp %s user. Error: %s', $exApp->getAppid(), $e->getMessage()), ['exception' => $e]);
+			return ['error' => 'Error while inserting ExApp user: ' . $e->getMessage()];
+		}
+		return $this->requestToExApp2($exApp, $route, $userId, $method, $queryParams, $bodyParams, $options, $request);
+	}
+
 	/**
 	 * Request to ExApp with AppAPI auth headers
 	 */
@@ -89,6 +112,23 @@ public function requestToExApp(
 		return $this->requestToExAppInternal($exApp, $method, $requestData['url'], $requestData['options']);
 	}
 
+	/**
+	 * Request to ExApp with AppAPI auth headers with proper query/body params handling
+	 */
+	public function requestToExApp2(
+		ExApp $exApp,
+		string $route,
+		?string $userId = null,
+		string $method = 'POST',
+		array $queryParams = [],
+		array $bodyParams = [],
+		array $options = [],
+		?IRequest $request = null,
+	): array|IResponse {
+		$requestData = $this->prepareRequestToExApp2($exApp, $route, $userId, $method, $queryParams, $bodyParams, $options, $request);
+		return $this->requestToExAppInternal($exApp, $method, $requestData['url'], $requestData['options']);
+	}
+
 	private function requestToExAppInternal(
 		ExApp $exApp,
 		string $method,
@@ -197,6 +237,7 @@ private function prepareRequestToExApp(
 		$options['nextcloud'] = [
 			'allow_local_address' => true, // it's required as we are using ExApp appid as hostname (usually local)
 		];
+		$options['http_errors'] = false; // do not throw exceptions on 4xx and 5xx responses
 		if (!empty($auth)) {
 			$options['auth'] = $auth;
 		}
@@ -211,6 +252,54 @@ private function prepareRequestToExApp(
 		return ['url' => $url, 'options' => $options];
 	}
 
+	private function prepareRequestToExApp2(
+		ExApp $exApp,
+		string $route,
+		?string $userId,
+		string $method,
+		array $queryParams,
+		array $bodyParams,
+		#[\SensitiveParameter]
+		array $options,
+		?IRequest $request,
+	): array {
+		$this->handleExAppDebug($exApp->getAppid(), $request, true);
+		$auth = [];
+		$url = $this->getExAppUrl($exApp, $exApp->getPort(), $auth);
+		if (str_starts_with($route, '/')) {
+			$url = $url.$route;
+		} else {
+			$url = $url.'/'.$route;
+		}
+
+		if (isset($options['headers']) && is_array($options['headers'])) {
+			$options['headers'] = [...$options['headers'], ...$this->commonService->buildAppAPIAuthHeaders($request, $userId, $exApp->getAppid(), $exApp->getVersion(), $exApp->getSecret())];
+		} else {
+			$options['headers'] = $this->commonService->buildAppAPIAuthHeaders($request, $userId, $exApp->getAppid(), $exApp->getVersion(), $exApp->getSecret());
+		}
+		$lang = $this->l10nFactory->findLanguage($exApp->getAppid());
+		if (!isset($options['headers']['Accept-Language'])) {
+			$options['headers']['Accept-Language'] = $lang;
+		}
+		$options['nextcloud'] = [
+			'allow_local_address' => true, // it's required as we are using ExApp appid as hostname (usually local)
+		];
+		$options['http_errors'] = false; // do not throw exceptions on 4xx and 5xx responses
+		if (!empty($auth)) {
+			$options['auth'] = $auth;
+		}
+
+		if ((!array_key_exists('multipart', $options))) {
+			if (count($queryParams) > 0) {
+				$url .= '?' . $this->getUriEncodedParams($queryParams);
+			}
+			if ($method !== 'GET' && count($bodyParams) > 0) {
+				$options['json'] = $bodyParams;
+			}
+		}
+		return ['url' => $url, 'options' => $options];
+	}
+
 	private function getUriEncodedParams(array $params): string {
 		$paramsContent = '';
 		foreach ($params as $key => $value) {

tests/psalm-baseline.xml

@@ -10,6 +10,19 @@
       <code>DavPlugin</code>
     </MissingDependency>
   </file>
+  <file src="lib/Controller/ExAppProxyController.php">
+    <UndefinedClass>
+      <code>CookieJar</code>
+      <code>CookieJar</code>
+      <code>RequestOptions</code>
+      <code>RequestOptions</code>
+      <code>RequestOptions</code>
+      <code>RequestOptions</code>
+      <code>RequestOptions</code>
+      <code>RequestOptions</code>
+      <code>SetCookie</code>
+    </UndefinedClass>
+  </file>
   <file src="lib/Controller/ExAppsPageController.php">
     <UndefinedClass>
       <code><![CDATA[$this->categoryFetcher]]></code>

tests/test_occ_commands_docker.py

@@ -43,43 +43,43 @@ def deploy_register():
 if __name__ == "__main__":
     # nextcloud_url should be without slash at the end
     register_daemon("http://127.0.0.1:8080/")
-    r = run("php occ app_api:daemon:list".split(), stdout=PIPE, stderr=PIPE, check=True)
+    r = run("php occ --no-warnings app_api:daemon:list".split(), stdout=PIPE, stderr=PIPE, check=True)
     assert not r.stderr.decode("UTF-8")
     r_output = r.stdout.decode("UTF-8")
     assert r_output.find("http://127.0.0.1:8080/") == -1
     assert r_output.find("http://127.0.0.1:8080") != -1
     unregister_daemon()
     # nextcloud_url should be without slash at the end but with "index.php"
     register_daemon("http://127.0.0.1:8080/index.php/")
-    r = run("php occ app_api:daemon:list".split(), stdout=PIPE, stderr=PIPE, check=True)
+    r = run("php occ --no-warnings app_api:daemon:list".split(), stdout=PIPE, stderr=PIPE, check=True)
     assert not r.stderr.decode("UTF-8")
     r_output = r.stdout.decode("UTF-8")
     assert r_output.find("http://127.0.0.1:8080/index.php/") == -1
     assert r_output.find("http://127.0.0.1:8080/index.php") != -1
     # silent should not fail, as there are not such ExApp
-    r = run("php occ app_api:app:unregister skeleton --silent".split(), stdout=PIPE, stderr=PIPE, check=True)
+    r = run("php occ --no-warnings app_api:app:unregister skeleton --silent".split(), stdout=PIPE, stderr=PIPE, check=True)
     assert not r.stderr.decode("UTF-8")
     r_output = r.stdout.decode("UTF-8")
     assert not r_output, f"Output should be empty: {r_output}"
     # without "--silent" it should fail, as there are not such ExApp
-    r = run("php occ app_api:app:unregister skeleton".split(), stdout=PIPE)
+    r = run("php occ --no-warnings app_api:app:unregister skeleton".split(), stdout=PIPE)
     assert r.returncode
     assert r.stdout.decode("UTF-8"), "Output should be non empty"
     # testing if "--keep-data" works.
     deploy_register()
-    r = run("php occ app_api:app:unregister skeleton --keep-data".split(), stdout=PIPE, check=True)
+    r = run("php occ --no-warnings app_api:app:unregister skeleton --keep-data".split(), stdout=PIPE, check=True)
     assert r.stdout.decode("UTF-8"), "Output should be non empty"
     run("docker volume inspect nc_app_skeleton_data".split(), check=True)
     # test if volume will be removed without "--keep-data"
     deploy_register()
-    run("php occ app_api:app:unregister skeleton".split(), check=True)
+    run("php occ --no-warnings app_api:app:unregister skeleton".split(), check=True)
     r = run("docker volume inspect nc_app_skeleton_data".split())
     assert r.returncode
     # test "--force" option
     deploy_register()
     run("docker container rm --force nc_app_skeleton".split(), check=True)
-    r = run("php occ app_api:app:unregister skeleton".split())
+    r = run("php occ --no-warnings app_api:app:unregister skeleton".split())
     assert r.returncode
-    r = run("php occ app_api:app:unregister skeleton --silent".split())
+    r = run("php occ --no-warnings app_api:app:unregister skeleton --silent".split())
     assert r.returncode
-    run("php occ app_api:app:unregister skeleton --force".split(), check=True)
+    run("php occ --no-warnings app_api:app:unregister skeleton --force".split(), check=True)