From a1136f4843c73a3e9c76134b153de2dc3aa02295 Mon Sep 17 00:00:00 2001 From: Andrey Borysenko Date: Tue, 21 May 2024 14:14:30 +0300 Subject: [PATCH 1/8] adjust proxy to pass cookies to ExApp Signed-off-by: Andrey Borysenko --- lib/Controller/ExAppProxyController.php | 38 ++++++++++++++++++++++--- tests/psalm-baseline.xml | 11 +++++++ 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/lib/Controller/ExAppProxyController.php b/lib/Controller/ExAppProxyController.php index 52b8de49..05a653db 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; @@ -77,7 +80,10 @@ public function ExAppGet(string $appId, string $other): Response { } $response = $this->service->requestToExApp( - $exApp, '/' . $other, $this->userId, 'GET', request: $this->request + $exApp, '/' . $other, $this->userId, 'GET', params: $_GET, options: [ + RequestOptions::COOKIES => $this->buildProxyCookiesJar($_COOKIE, $exApp->getHost()), + ], + request: $this->request, ); if (is_array($response)) { return (new Response())->setStatus(500); @@ -95,7 +101,10 @@ public function ExAppPost(string $appId, string $other): Response { $response = $this->service->aeRequestToExApp( $exApp, '/' . $other, $this->userId, - params: $this->request->getParams(), request: $this->request + params: $this->request->getParams(), options: [ + RequestOptions::COOKIES => $this->buildProxyCookiesJar($_COOKIE, $exApp->getHost()), + ], + request: $this->request, ); if (is_array($response)) { return (new Response())->setStatus(500); @@ -112,7 +121,10 @@ public function ExAppPut(string $appId, string $other): Response { } $response = $this->service->aeRequestToExApp( - $exApp, '/' . $other, $this->userId, 'PUT', $this->request->getParams(), request: $this->request + $exApp, '/' . $other, $this->userId, 'PUT', $this->request->getParams(), options: [ + RequestOptions::COOKIES => $this->buildProxyCookiesJar($_COOKIE, $exApp->getHost()), + ], + request: $this->request, ); if (is_array($response)) { return (new Response())->setStatus(500); @@ -129,11 +141,29 @@ public function ExAppDelete(string $appId, string $other): Response { } $response = $this->service->aeRequestToExApp( - $exApp, '/' . $other, $this->userId, 'DELETE', $this->request->getParams(), request: $this->request + $exApp, '/' . $other, $this->userId, 'DELETE', $this->request->getParams(), 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; + } } diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index c019c0fd..f4482689 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -10,6 +10,17 @@ DavPlugin + + + CookieJar + CookieJar + RequestOptions + RequestOptions + RequestOptions + RequestOptions + SetCookie + + categoryFetcher]]> From fa5e9f61b05d44caf3af6e3e585aa188d7f58201 Mon Sep 17 00:00:00 2001 From: Andrey Borysenko Date: Thu, 23 May 2024 19:19:50 +0300 Subject: [PATCH 2/8] proxy fixes after local tests Signed-off-by: Andrey Borysenko --- lib/Controller/ExAppProxyController.php | 73 +++++++++++++++++---- lib/Service/AppAPIService.php | 87 +++++++++++++++++++++++++ 2 files changed, 147 insertions(+), 13 deletions(-) diff --git a/lib/Controller/ExAppProxyController.php b/lib/Controller/ExAppProxyController.php index 05a653db..746e7679 100644 --- a/lib/Controller/ExAppProxyController.php +++ b/lib/Controller/ExAppProxyController.php @@ -65,7 +65,8 @@ 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') { $proxyResponse->cacheFor(3600); } return $proxyResponse; @@ -79,8 +80,8 @@ public function ExAppGet(string $appId, string $other): Response { return new NotFoundResponse(); } - $response = $this->service->requestToExApp( - $exApp, '/' . $other, $this->userId, 'GET', params: $_GET, options: [ + $response = $this->service->aeRequestToExApp2( + $exApp, '/' . $other, $this->userId, 'GET', queryParams: $_GET, options: [ RequestOptions::COOKIES => $this->buildProxyCookiesJar($_COOKIE, $exApp->getHost()), ], request: $this->request, @@ -99,11 +100,18 @@ 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(), options: [ - RequestOptions::COOKIES => $this->buildProxyCookiesJar($_COOKIE, $exApp->getHost()), - ], + queryParams: $_GET, bodyParams: $bodyParams, options: $options, request: $this->request, ); if (is_array($response)) { @@ -120,10 +128,18 @@ public function ExAppPut(string $appId, string $other): Response { return new NotFoundResponse(); } - $response = $this->service->aeRequestToExApp( - $exApp, '/' . $other, $this->userId, 'PUT', $this->request->getParams(), options: [ - RequestOptions::COOKIES => $this->buildProxyCookiesJar($_COOKIE, $exApp->getHost()), - ], + $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)) { @@ -140,8 +156,10 @@ public function ExAppDelete(string $appId, string $other): Response { return new NotFoundResponse(); } - $response = $this->service->aeRequestToExApp( - $exApp, '/' . $other, $this->userId, 'DELETE', $this->request->getParams(), options: [ + $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, @@ -166,4 +184,33 @@ private function buildProxyCookiesJar(array $cookies, string $domain): CookieJar } 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..69ca972b 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, @@ -211,6 +251,53 @@ 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) + ]; + 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) { From 060efdafd53fa4911a4d2ef6364173f160fea512 Mon Sep 17 00:00:00 2001 From: Andrey Borysenko Date: Thu, 23 May 2024 19:28:06 +0300 Subject: [PATCH 3/8] calm psalm Signed-off-by: Andrey Borysenko --- tests/psalm-baseline.xml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index f4482689..c94c15eb 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -18,6 +18,8 @@ RequestOptions RequestOptions RequestOptions + RequestOptions + RequestOptions SetCookie From 54618963c89e523378f43d0d050f040d94361271 Mon Sep 17 00:00:00 2001 From: Andrey Borysenko Date: Thu, 23 May 2024 19:31:17 +0300 Subject: [PATCH 4/8] php-cs Signed-off-by: Andrey Borysenko --- lib/Controller/ExAppProxyController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Controller/ExAppProxyController.php b/lib/Controller/ExAppProxyController.php index 746e7679..51304547 100644 --- a/lib/Controller/ExAppProxyController.php +++ b/lib/Controller/ExAppProxyController.php @@ -66,7 +66,7 @@ private function createProxyResponse(string $path, IResponse $response, $cache = $proxyResponse = new ProxyResponse($response->getStatusCode(), $responseHeaders, $content); if ($cache && !$isHTML && empty($response->getHeader('cache-control')) - && $response->getHeader ('Content-Type') !== 'application/json') { + && $response->getHeader('Content-Type') !== 'application/json') { $proxyResponse->cacheFor(3600); } return $proxyResponse; From 127b830992240edc787054bd5f415b33ed827714 Mon Sep 17 00:00:00 2001 From: Andrey Borysenko Date: Sat, 1 Jun 2024 16:27:54 +0300 Subject: [PATCH 5/8] set http_errors to false, do not throw exception Signed-off-by: Andrey Borysenko --- lib/Service/AppAPIService.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/Service/AppAPIService.php b/lib/Service/AppAPIService.php index 69ca972b..c9c1ffd1 100644 --- a/lib/Service/AppAPIService.php +++ b/lib/Service/AppAPIService.php @@ -237,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; } @@ -283,6 +284,7 @@ private function prepareRequestToExApp2( $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; } From e3351ebfcd7f616b485ede31cebda6eda30d6ab4 Mon Sep 17 00:00:00 2001 From: Alexander Piskun <13381981+bigcat88@users.noreply.github.com> Date: Tue, 4 Jun 2024 11:50:10 +0300 Subject: [PATCH 6/8] allow proxying of "apps/app_api/proxy/ex_app_id/" (root url for ExApp) Signed-off-by: Alexander Piskun --- appinfo/routes.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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' => '']], From 4429be425975091c869164b77cd51a57cbb3fa3a Mon Sep 17 00:00:00 2001 From: Alexander Piskun <13381981+bigcat88@users.noreply.github.com> Date: Mon, 10 Jun 2024 20:27:43 +0300 Subject: [PATCH 7/8] fixed CI Signed-off-by: Alexander Piskun --- tests/test_occ_commands_docker.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) 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) From 80878b1f8b50d7c47a0b694dcaf502059693c8eb Mon Sep 17 00:00:00 2001 From: Alexander Piskun <13381981+bigcat88@users.noreply.github.com> Date: Mon, 10 Jun 2024 20:40:51 +0300 Subject: [PATCH 8/8] UIProxy: do not cache "application/x-tar" mimetype Signed-off-by: Alexander Piskun --- lib/Controller/ExAppProxyController.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Controller/ExAppProxyController.php b/lib/Controller/ExAppProxyController.php index 51304547..48a75b84 100644 --- a/lib/Controller/ExAppProxyController.php +++ b/lib/Controller/ExAppProxyController.php @@ -66,7 +66,8 @@ private function createProxyResponse(string $path, IResponse $response, $cache = $proxyResponse = new ProxyResponse($response->getStatusCode(), $responseHeaders, $content); if ($cache && !$isHTML && empty($response->getHeader('cache-control')) - && $response->getHeader('Content-Type') !== 'application/json') { + && $response->getHeader('Content-Type') !== 'application/json' + && $response->getHeader('Content-Type') !== 'application/x-tar') { $proxyResponse->cacheFor(3600); } return $proxyResponse;