diff --git a/docs/configuration.rst b/docs/configuration.rst index 6d4bb24e..d437cd03 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -106,8 +106,10 @@ Web server settings. If any of ``host``/``port`` is not set, web integration wil * ``debug`` - enable debug toolbar, boolean, optional, default ``no``. * ``debug_check_host`` - check hosts to access debug toolbar, boolean, optional, default ``no``. * ``debug_allowed_hosts`` - allowed hosts to get access to debug toolbar, space separated list of string, optional. +* ``enable_archive_upload`` - allow to upload packages via HTTP (i.e. call of ``/api/v1/service/upload`` uri), boolean, optional, default ``no``. * ``host`` - host to bind, string, optional. * ``index_url`` - full url of the repository index page, string, optional. +* ``max_body_size`` - max body size in bytes to be validated for archive upload, integer, optional. If not set, validation will be disabled. * ``password`` - password to authorize in web service in order to update service status, string, required in case if authorization enabled. * ``port`` - port to bind, int, optional. * ``static_path`` - path to directory with static files, string, required. diff --git a/docs/faq.rst b/docs/faq.rst index 9cf29383..e9f498e4 100644 --- a/docs/faq.rst +++ b/docs/faq.rst @@ -773,7 +773,14 @@ In this example the following settings are assumed: Master node configuration """"""""""""""""""""""""" -The only requirements for the master node is that API must be available for worker nodes to call (e.g. port must be exposed to internet, or local network in case of VPN, etc). In addition, the following settings are recommended: +The only requirements for the master node is that API must be available for worker nodes to call (e.g. port must be exposed to internet, or local network in case of VPN, etc). It is also required to allow file upload: + +.. code-block:: ini + + [web] + enable_archive_upload = yes + +In addition, the following settings are recommended: * As it has been mentioned above, it is recommended to enable authentication (see `How to enable basic authorization`_) and create system user which will be used later. Later this user (if any) will be referenced as ``worker-user``. @@ -848,6 +855,9 @@ Master node config (``master.ini``) as: [auth] target = mapping + [web] + enable_archive_upload = yes + Command to run master node: .. code-block:: shell diff --git a/src/ahriman/core/configuration/schema.py b/src/ahriman/core/configuration/schema.py index d3cb9f21..1dfc833d 100644 --- a/src/ahriman/core/configuration/schema.py +++ b/src/ahriman/core/configuration/schema.py @@ -228,6 +228,10 @@ CONFIGURATION_SCHEMA: ConfigurationSchema = { "coerce": "list", "schema": {"type": "string"}, }, + "enable_archive_upload": { + "type": "boolean", + "coerce": "boolean", + }, "host": { "type": "string", "is_ip_address": ["localhost"], @@ -236,6 +240,11 @@ CONFIGURATION_SCHEMA: ConfigurationSchema = { "type": "string", "is_url": ["http", "https"], }, + "max_body_size": { + "type": "integer", + "coerce": "integer", + "min": 0, + }, "password": { "type": "string", }, diff --git a/src/ahriman/core/status/web_client.py b/src/ahriman/core/status/web_client.py index 882fa860..34371793 100644 --- a/src/ahriman/core/status/web_client.py +++ b/src/ahriman/core/status/web_client.py @@ -22,6 +22,7 @@ import logging import requests from collections.abc import Generator +from functools import cached_property from typing import Any, IO, Literal from urllib.parse import quote_plus as urlencode @@ -44,6 +45,7 @@ class WebClient(Client, LazyLogging): address(str): address of the web service suppress_errors(bool): suppress logging errors (e.g. if no web server available) user(User | None): web service user descriptor + use_unix_socket(bool): use websocket or not """ _login_url = "/api/v1/login" @@ -56,13 +58,21 @@ class WebClient(Client, LazyLogging): Args: configuration(Configuration): configuration instance """ - self.address, use_unix_socket = self.parse_address(configuration) + self.address, self.use_unix_socket = self.parse_address(configuration) self.user = User.from_option( configuration.get("web", "username", fallback=None), configuration.get("web", "password", fallback=None)) self.suppress_errors = configuration.getboolean("settings", "suppress_http_log_errors", fallback=False) - self.__session = self._create_session(use_unix_socket=use_unix_socket) + @cached_property + def session(self) -> requests.Session: + """ + get or create session + + Returns: + request.Session: created session object + """ + return self._create_session(use_unix_socket=self.use_unix_socket) @staticmethod def _logs_url(package_base: str) -> str: @@ -130,7 +140,7 @@ class WebClient(Client, LazyLogging): if session is not None: yield session # use session from arguments else: - yield self.__session # use instance generated session + yield self.session # use instance generated session except requests.RequestException as e: if self.suppress_errors: return diff --git a/src/ahriman/core/upload/http_upload.py b/src/ahriman/core/upload/http_upload.py index df423292..69eb6334 100644 --- a/src/ahriman/core/upload/http_upload.py +++ b/src/ahriman/core/upload/http_upload.py @@ -20,6 +20,7 @@ import hashlib import requests +from functools import cached_property from pathlib import Path from typing import Any @@ -52,6 +53,16 @@ class HttpUpload(Upload): self.auth = (password, username) if password and username else None self.timeout = configuration.getint(section, "timeout", fallback=30) + @cached_property + def session(self) -> requests.Session: + """ + get or create session + + Returns: + request.Session: created session object + """ + return requests.Session() + @staticmethod def calculate_hash(path: Path) -> str: """ @@ -110,7 +121,7 @@ class HttpUpload(Upload): requests.Response: request response object """ try: - response = requests.request(method, url, auth=self.auth, timeout=self.timeout, **kwargs) + response = self.session.request(method, url, auth=self.auth, timeout=self.timeout, **kwargs) response.raise_for_status() except requests.HTTPError as e: self.logger.exception("could not perform %s request to %s: %s", method, url, exception_response_text(e)) diff --git a/src/ahriman/core/upload/remote_service.py b/src/ahriman/core/upload/remote_service.py index 4fb66c2d..e6eeea30 100644 --- a/src/ahriman/core/upload/remote_service.py +++ b/src/ahriman/core/upload/remote_service.py @@ -17,16 +17,19 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . # +import requests + +from functools import cached_property from pathlib import Path from typing import IO from ahriman.core.configuration import Configuration from ahriman.core.status.web_client import WebClient -from ahriman.core.upload.upload import Upload +from ahriman.core.upload.http_upload import HttpUpload from ahriman.models.package import Package -class RemoteService(Upload): +class RemoteService(HttpUpload): """ upload files to another server instance @@ -43,11 +46,19 @@ class RemoteService(Upload): configuration(Configuration): configuration instance section(str): settings section name """ - Upload.__init__(self, architecture, configuration) - del section - + HttpUpload.__init__(self, architecture, configuration, section) self.client = WebClient(configuration) + @cached_property + def session(self) -> requests.Session: + """ + get or create session + + Returns: + request.Session: created session object + """ + return self.client.session + def package_upload(self, path: Path, package: Package) -> None: """ upload single package to remote @@ -66,7 +77,7 @@ class RemoteService(Upload): part: tuple[str, IO[bytes], str, dict[str, str]] = ( descriptor.filename, archive, "application/octet-stream", {} ) - self.client.make_request("POST", "/api/v1/service/upload", files={"archive": part}) + self._request("POST", f"{self.client.address}/api/v1/service/upload", files={"archive": part}) def sync(self, path: Path, built_packages: list[Package]) -> None: """ diff --git a/src/ahriman/web/views/service/process.py b/src/ahriman/web/views/service/process.py index 7a1c70d0..c27057f8 100644 --- a/src/ahriman/web/views/service/process.py +++ b/src/ahriman/web/views/service/process.py @@ -44,7 +44,7 @@ class ProcessView(BaseView): 200: {"description": "Success response", "schema": ProcessSchema}, 401: {"description": "Authorization required", "schema": ErrorSchema}, 403: {"description": "Access is forbidden", "schema": ErrorSchema}, - 404: {"description": "Process ID is unknown", "schema": ErrorSchema}, + 404: {"description": "Not found", "schema": ErrorSchema}, 500: {"description": "Internal server error", "schema": ErrorSchema}, }, security=[{"token": [GET_PERMISSION]}], diff --git a/src/ahriman/web/views/service/upload.py b/src/ahriman/web/views/service/upload.py index 232862a6..86d89a24 100644 --- a/src/ahriman/web/views/service/upload.py +++ b/src/ahriman/web/views/service/upload.py @@ -20,7 +20,7 @@ import aiohttp_apispec # type: ignore[import] from aiohttp import BodyPartReader -from aiohttp.web import HTTPBadRequest, HTTPCreated +from aiohttp.web import HTTPBadRequest, HTTPCreated, HTTPNotFound from pathlib import Path from ahriman.models.user_access import UserAccess @@ -47,6 +47,7 @@ class UploadView(BaseView): 400: {"description": "Bad data is supplied", "schema": ErrorSchema}, 401: {"description": "Authorization required", "schema": ErrorSchema}, 403: {"description": "Access is forbidden", "schema": ErrorSchema}, + 404: {"description": "Not found", "schema": ErrorSchema}, 500: {"description": "Internal server error", "schema": ErrorSchema}, }, security=[{"token": [POST_PERMISSION]}], @@ -61,6 +62,9 @@ class UploadView(BaseView): HTTPBadRequest: if bad data is supplied HTTPCreated: on success response """ + if not self.configuration.getboolean("web", "enable_archive_upload", fallback=False): + raise HTTPNotFound() + try: reader = await self.request.multipart() except Exception as e: @@ -81,12 +85,20 @@ class UploadView(BaseView): if Path(archive_name).resolve().name != archive_name: raise HTTPBadRequest(reason="Filename must be valid archive name") + max_body_size = self.configuration.getint("web", "max_body_size", fallback=None) output = self.configuration.repository_paths.packages / archive_name + current_size = 0 + with output.open("wb") as archive: while True: chunk = await part.read_chunk() if not chunk: break + + current_size += len(chunk) + if max_body_size is not None and current_size > max_body_size: + raise HTTPBadRequest(reason="Body part is too large") + archive.write(chunk) raise HTTPCreated() diff --git a/tests/ahriman/core/upload/test_http_upload.py b/tests/ahriman/core/upload/test_http_upload.py index 544c5562..07dbb23f 100644 --- a/tests/ahriman/core/upload/test_http_upload.py +++ b/tests/ahriman/core/upload/test_http_upload.py @@ -47,7 +47,7 @@ def test_request(github: Github, mocker: MockerFixture) -> None: must call request method """ response_mock = MagicMock() - request_mock = mocker.patch("requests.request", return_value=response_mock) + request_mock = mocker.patch("requests.Session.request", return_value=response_mock) github._request("GET", "url", arg="arg") request_mock.assert_called_once_with("GET", "url", auth=github.auth, timeout=github.timeout, arg="arg") @@ -58,6 +58,6 @@ def test_request_exception(github: Github, mocker: MockerFixture) -> None: """ must call request method and log HTTPError exception """ - mocker.patch("requests.request", side_effect=requests.HTTPError()) + mocker.patch("requests.Session.request", side_effect=requests.HTTPError()) with pytest.raises(requests.HTTPError): github._request("GET", "url", arg="arg") diff --git a/tests/ahriman/core/upload/test_remote_service.py b/tests/ahriman/core/upload/test_remote_service.py index 0b4879bd..179e239b 100644 --- a/tests/ahriman/core/upload/test_remote_service.py +++ b/tests/ahriman/core/upload/test_remote_service.py @@ -7,17 +7,26 @@ from ahriman.core.upload.remote_service import RemoteService from ahriman.models.package import Package +def test_session(remote_service: RemoteService, mocker: MockerFixture) -> None: + """ + must generate ahriman session + """ + upload_mock = mocker.patch("ahriman.core.status.web_client.WebClient._create_session") + assert remote_service.session + upload_mock.assert_called_once_with(use_unix_socket=False) + + def test_package_upload(remote_service: RemoteService, package_ahriman: Package, mocker: MockerFixture) -> None: """ must upload package to remote host """ open_mock = mocker.patch("pathlib.Path.open") - upload_mock = mocker.patch("ahriman.core.status.web_client.WebClient.make_request") + upload_mock = mocker.patch("ahriman.core.upload.http_upload.HttpUpload._request") filename = package_ahriman.packages[package_ahriman.base].filename remote_service.sync(Path("local"), [package_ahriman]) open_mock.assert_called_once_with("rb") - upload_mock.assert_called_once_with("POST", "/api/v1/service/upload", files={ + upload_mock.assert_called_once_with("POST", f"{remote_service.client.address}/api/v1/service/upload", files={ "archive": (filename, pytest.helpers.anyvar(int), "application/octet-stream", {}) }) diff --git a/tests/ahriman/web/views/service/test_views_service_upload.py b/tests/ahriman/web/views/service/test_views_service_upload.py index ed2fe8c9..971bdb01 100644 --- a/tests/ahriman/web/views/service/test_views_service_upload.py +++ b/tests/ahriman/web/views/service/test_views_service_upload.py @@ -33,59 +33,81 @@ async def test_post(client: TestClient, mocker: MockerFixture) -> None: open_mock.assert_called_once_with("wb") +async def test_post_not_found(client: TestClient, mocker: MockerFixture) -> None: + """ + must return 404 if request was disabled + """ + mocker.patch("ahriman.core.configuration.Configuration.getboolean", return_value=False) + data = FormData() + data.add_field("archive", BytesIO(b"content"), filename="filename", content_type="application/octet-stream") + response_schema = pytest.helpers.schema_response(UploadView.post, code=404) + + response = await client.post("/api/v1/service/upload", data=data) + assert response.status == 404 + assert not response_schema.validate(await response.json()) + + async def test_post_not_multipart(client: TestClient) -> None: """ must return 400 on invalid payload """ + response_schema = pytest.helpers.schema_response(UploadView.post, code=400) + response = await client.post("/api/v1/service/upload") assert response.status == 400 + assert not response_schema.validate(await response.json()) -async def test_post_not_bodypart(client: TestClient, mocker: MockerFixture) -> None: +async def test_post_not_body_part(client: TestClient, mocker: MockerFixture) -> None: """ must return 400 on invalid iterator in multipart """ + response_schema = pytest.helpers.schema_response(UploadView.post, code=400) mocker.patch("aiohttp.MultipartReader.next", return_value=42) # surprise, motherfucker data = FormData() data.add_field("archive", BytesIO(b"content"), filename="filename", content_type="application/octet-stream") response = await client.post("/api/v1/service/upload", data=data) assert response.status == 400 + assert not response_schema.validate(await response.json()) async def test_post_not_archive(client: TestClient) -> None: """ must return 400 on invalid multipart key """ + response_schema = pytest.helpers.schema_response(UploadView.post, code=400) data = FormData() data.add_field("random", BytesIO(b"content"), filename="filename", content_type="application/octet-stream") response = await client.post("/api/v1/service/upload", data=data) assert response.status == 400 - - -async def test_post_no_filename(client: TestClient, mocker: MockerFixture) -> None: - """ - must return 400 if filename is not set - """ - mocker.patch("aiohttp.BodyPartReader.filename", return_value=None) - data = FormData() - data.add_field("random", BytesIO(b"content"), filename="filename", content_type="application/octet-stream") - - response = await client.post("/api/v1/service/upload", data=data) - assert response.status == 400 + assert not response_schema.validate(await response.json()) async def test_post_filename_invalid(client: TestClient) -> None: """ must return 400 if filename is invalid """ + response_schema = pytest.helpers.schema_response(UploadView.post, code=400) + data = FormData() data.add_field("archive", BytesIO(b"content"), filename="..", content_type="application/octet-stream") response = await client.post("/api/v1/service/upload", data=data) assert response.status == 400 + assert not response_schema.validate(await response.json()) + +async def test_post_file_too_big(client: TestClient, mocker: MockerFixture) -> None: + """ + must return 400 if file is too big + """ + mocker.patch("pathlib.Path.open") + mocker.patch("ahriman.core.configuration.Configuration.getint", return_value=0) data = FormData() - data.add_field("archive", BytesIO(b"content"), filename="", content_type="application/octet-stream") + data.add_field("archive", BytesIO(b"content"), filename="filename", content_type="application/octet-stream") + response_schema = pytest.helpers.schema_response(UploadView.post, code=400) + response = await client.post("/api/v1/service/upload", data=data) assert response.status == 400 + assert not response_schema.validate(await response.json()) diff --git a/tests/testresources/core/ahriman.ini b/tests/testresources/core/ahriman.ini index 74d87690..d12b4c3b 100644 --- a/tests/testresources/core/ahriman.ini +++ b/tests/testresources/core/ahriman.ini @@ -112,6 +112,7 @@ username = arcan1s debug = no debug_check_host = no debug_allowed_hosts = +enable_archive_upload = yes host = 127.0.0.1 static_path = ../web/templates/static templates = ../web/templates \ No newline at end of file