diff --git a/appinfo/routes.php b/appinfo/routes.php index fabe2598..ea911374 100644 --- a/appinfo/routes.php +++ b/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' => '']], diff --git a/lib/Controller/ExAppProxyController.php b/lib/Controller/ExAppProxyController.php index 52b8de49..48a75b84 100644 --- a/lib/Controller/ExAppProxyController.php +++ b/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; + } } diff --git a/lib/Service/AppAPIService.php b/lib/Service/AppAPIService.php index c0a5cc7a..c9c1ffd1 100644 --- a/lib/Service/AppAPIService.php +++ b/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) { diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index c019c0fd..c94c15eb 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -10,6 +10,19 @@ DavPlugin + + + CookieJar + CookieJar + RequestOptions + RequestOptions + RequestOptions + RequestOptions + RequestOptions + RequestOptions + SetCookie + + categoryFetcher]]> diff --git a/tests/test_occ_commands_docker.py b/tests/test_occ_commands_docker.py index 909830ae..e4492a89 100644 --- a/tests/test_occ_commands_docker.py +++ b/tests/test_occ_commands_docker.py @@ -43,7 +43,7 @@ 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 @@ -51,35 +51,35 @@ def deploy_register(): 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)