From ac17ceb3099465f93342fb904b20655fdaf3c79f Mon Sep 17 00:00:00 2001 From: sh-andriy <105591819+sh-andriy@users.noreply.github.com> Date: Mon, 2 Dec 2024 15:45:28 +0200 Subject: [PATCH] [ENG-6569] | wb_config fix for Owncloud and Bitbucket + Owncloud Implementation Connection Fix bug (#167) --- addon_imps/storage/bitbucket.py | 45 ++++---- addon_imps/storage/owncloud.py | 121 ++++++++++++--------- addon_imps/tests/storage/test_bitbucket.py | 6 +- addon_service/common/waterbutler_compat.py | 26 ++++- 4 files changed, 116 insertions(+), 82 deletions(-) diff --git a/addon_imps/storage/bitbucket.py b/addon_imps/storage/bitbucket.py index e01c11b1..ca455d8e 100644 --- a/addon_imps/storage/bitbucket.py +++ b/addon_imps/storage/bitbucket.py @@ -39,6 +39,29 @@ async def get_external_account_id(self, auth_result_extras: dict[str, str]) -> s raise ValueError("Failed to retrieve user UUID") return uuid + async def build_wb_config(self) -> dict: + if not self.config.connected_root_id: + raise ValueError( + "connected_root_id is not set. Cannot build WaterButler config." + ) + item_type_str, actual_id = self._parse_item_id(self.config.connected_root_id) + if item_type_str == "repository": + workspace_slug, repo_slug = actual_id.split("/", 1) + return { + "owner": workspace_slug, + "repo": repo_slug, + "host": "api.bitbucket.org", + } + elif item_type_str == "workspace": + return { + "owner": actual_id, + "host": "api.bitbucket.org", + } + else: + raise ValueError( + f"Unsupported item type for build_wb_config: {item_type_str}" + ) + async def list_root_items(self, page_cursor: str = "") -> storage.ItemSampleResult: params = self._params_from_cursor(page_cursor) params["pagelen"] = "100" @@ -236,25 +259,3 @@ async def _handle_response(self, response) -> dict: error_message = json_data.get("error", {}).get("message", "Unknown error") raise ValueError(f"HTTP Error {response.http_status}: {error_message}") return await response.json_content() - - async def build_wb_config(self) -> dict: - item_type_str, actual_id = self._parse_item_id(self.config.connected_root_id) - if item_type_str == "repository": - workspace_slug, repo_slug = actual_id.split("/", 1) - host = urlparse(self.config.external_api_url).hostname - return { - "workspace": workspace_slug, - "repo_slug": repo_slug, - "host": host, - } - elif item_type_str == "workspace": - workspace_slug = actual_id - host = urlparse(self.config.external_api_url).hostname - return { - "workspace": workspace_slug, - "host": host, - } - else: - raise ValueError( - f"Unsupported item type for build_wb_config: {item_type_str}" - ) diff --git a/addon_imps/storage/owncloud.py b/addon_imps/storage/owncloud.py index 0277fbc7..afb943b2 100644 --- a/addon_imps/storage/owncloud.py +++ b/addon_imps/storage/owncloud.py @@ -34,32 +34,43 @@ async def get_external_account_id(self, auth_result_extras: dict[str, str]) -> s "Depth": "0", } async with self.network.PROPFIND( - uri_path="", + uri_path=self._strip_absolute_path(""), headers=headers, content=_BUILD_PROPFIND_CURRENT_USER_PRINCIPAL, ) as response: response_xml = await response.text_content() - current_user_principal_url = self._parse_current_user_principal( - response_xml - ) + try: + current_user_principal_url = self._parse_current_user_principal( + response_xml + ) + except ValueError: + username = auth_result_extras.get("username") or self._fallback_username + if not username: + raise ValueError( + "Username is required for fallback but not provided." + ) + current_user_principal_url = f"/remote.php/dav/files/{username}/" current_user_principal_url = current_user_principal_url.lstrip("/") async with self.network.PROPFIND( - uri_path=current_user_principal_url, + uri_path=self._strip_absolute_path(current_user_principal_url), headers=headers, content=_BUILD_PROPFIND_DISPLAYNAME, ) as response: response_xml = await response.text_content() - displayname = self._parse_displayname(response_xml) - return displayname + return self._parse_displayname(response_xml) + + @property + def _fallback_username(self) -> str | None: + return "default-username" async def list_root_items(self, page_cursor: str = "") -> storage.ItemSampleResult: return await self.list_child_items(_owncloud_root_id(), page_cursor) async def get_item_info(self, item_id: str) -> storage.ItemResult: item_type, path = _parse_item_id(item_id) - url = self._build_url(path) + url = self._strip_absolute_path(path) headers = { "Depth": "0", @@ -87,14 +98,13 @@ async def list_child_items( item_type: storage.ItemType | None = None, ) -> storage.ItemSampleResult: _item_type, path = _parse_item_id(item_id) - url = self._build_url(path) - + relative_path = self._strip_absolute_path(path) headers = { "Depth": "1", } async with self.network.PROPFIND( - uri_path=url, + uri_path=relative_path, headers=headers, content=_BUILD_PROPFIND_ALLPROPS, ) as response: @@ -119,46 +129,45 @@ async def list_child_items( return storage.ItemSampleResult(items=items) + def _strip_absolute_path(self, path: str) -> str: + return path.lstrip("/") + async def build_wb_config(self) -> dict: - return { - "folder": self.config.connected_root_id, - "host": self.config.external_api_url, + base_url = self.config.external_api_url.rstrip("/") + parsed_url = urlparse(base_url) + + root_host = f"{parsed_url.scheme}://{parsed_url.netloc}" + folder_path = "" + + wb_config = { + "folder": folder_path, + "host": root_host, + "verify_ssl": True, } + return wb_config def _parse_response_element( self, response_element: ET.Element, path: str ) -> storage.ItemResult: ns = {"d": "DAV:", "oc": "http://owncloud.org/ns"} resourcetype = response_element.find(".//d:resourcetype", ns) - if ( - resourcetype is not None + item_type = ( + storage.ItemType.FOLDER + if resourcetype is not None and resourcetype.find("d:collection", ns) is not None - ): - item_type = storage.ItemType.FOLDER - else: - item_type = storage.ItemType.FILE - + else storage.ItemType.FILE + ) displayname_element = response_element.find(".//d:displayname", ns) - if displayname_element is not None and displayname_element.text: - displayname = displayname_element.text - else: - displayname = path.rstrip("/").split("/")[-1] - - item_result = storage.ItemResult( + displayname = ( + displayname_element.text + if displayname_element is not None and displayname_element.text + else path.rstrip("/").split("/")[-1] + ) + return storage.ItemResult( item_id=_make_item_id(item_type, path), item_name=displayname, item_type=item_type, ) - return item_result - - def _parse_property(self, response_xml: str, xpath: str, error_message: str) -> str: - ns = {"d": "DAV:"} - root = ET.fromstring(response_xml) - element = root.find(xpath, ns) - if element is not None and element.text: - return element.text - else: - raise ValueError(error_message) def _parse_current_user_principal(self, response_xml: str) -> str: return self._parse_property( @@ -168,14 +177,23 @@ def _parse_current_user_principal(self, response_xml: str) -> str: ) def _parse_displayname(self, response_xml: str) -> str: - return self._parse_property( - response_xml, - xpath=".//d:displayname", - error_message="displayname not found in response", - ) + try: + return self._parse_property( + response_xml, + xpath=".//d:displayname", + error_message="displayname not found in response", + ) + except ValueError: + return "default-name" - def _build_url(self, path: str) -> str: - return path.lstrip("/") + def _parse_property(self, response_xml: str, xpath: str, error_message: str) -> str: + ns = {"d": "DAV:"} + root = ET.fromstring(response_xml) + element = root.find(xpath, ns) + if element is not None and element.text: + return element.text + else: + raise ValueError(error_message) def _href_to_path(self, href: str) -> str: parsed_href = urlparse(unquote(href)) @@ -187,6 +205,8 @@ def _href_to_path(self, href: str) -> str: path = href_path[len(base_path):] else: path = href_path + + path = path.strip("/") return path or "/" @@ -195,15 +215,10 @@ def _make_item_id(item_type: storage.ItemType, path: str) -> str: def _parse_item_id(item_id: str) -> tuple[storage.ItemType, str]: - try: - if not item_id: - return ItemType.FOLDER, "/" - (_type, _path) = item_id.split(":", maxsplit=1) - return (storage.ItemType(_type), _path) - except ValueError: - raise ValueError( - f'Expected id of format "type:path", e.g. "FOLDER:/path/to/folder" (got "{item_id}")' - ) + if not item_id: + return ItemType.FOLDER, "/" + _type, _path = item_id.split(":", maxsplit=1) + return storage.ItemType(_type), _path def _owncloud_root_id() -> str: diff --git a/addon_imps/tests/storage/test_bitbucket.py b/addon_imps/tests/storage/test_bitbucket.py index c2515c93..8ed99a26 100644 --- a/addon_imps/tests/storage/test_bitbucket.py +++ b/addon_imps/tests/storage/test_bitbucket.py @@ -311,8 +311,8 @@ async def test_build_wb_config_repository(self): result = await self.imp.build_wb_config() expected_result = { - "workspace": self.WORKSPACE, - "repo_slug": self.REPO, + "owner": self.WORKSPACE, + "repo": self.REPO, "host": "api.bitbucket.org", } self.assertEqual(result, expected_result) @@ -328,7 +328,7 @@ async def test_build_wb_config_workspace(self): result = await self.imp.build_wb_config() expected_result = { - "workspace": self.WORKSPACE, + "owner": self.WORKSPACE, "host": "api.bitbucket.org", } self.assertEqual(result, expected_result) diff --git a/addon_service/common/waterbutler_compat.py b/addon_service/common/waterbutler_compat.py index 48a67e48..c16d96f6 100644 --- a/addon_service/common/waterbutler_compat.py +++ b/addon_service/common/waterbutler_compat.py @@ -1,3 +1,5 @@ +import functools + from asgiref.sync import async_to_sync from rest_framework_json_api import serializers @@ -24,22 +26,38 @@ class JSONAPIMeta: def _credentials_for_waterbutler(self, configured_storage_addon): _creds_data = configured_storage_addon.credentials + imp = get_storage_addon_instance__blocking( + configured_storage_addon.imp_cls, + configured_storage_addon.base_account, + configured_storage_addon.config, + ) + wb_config = async_to_sync(imp.build_wb_config)() + match type(_creds_data): case credentials.AccessTokenCredentials: - return {"token": _creds_data.access_token} + creds = {"token": _creds_data.access_token} + if "host" in wb_config: + creds["host"] = wb_config["host"] + return creds case ( credentials.AccessKeySecretKeyCredentials | credentials.UsernamePasswordCredentials ): # field names line up with waterbutler's expectations - return json_arguments.json_for_dataclass(_creds_data) + serialized_creds = json_arguments.json_for_dataclass(_creds_data) + if "host" in wb_config: + serialized_creds["host"] = wb_config["host"] + return serialized_creds case _: raise ValueError(f"unknown credentials type: {_creds_data}") - def _config_for_waterbutler(self, configured_storage_addon: ConfiguredStorageAddon): + @staticmethod + @functools.cache + def _config_for_waterbutler(configured_storage_addon: ConfiguredStorageAddon): imp = get_storage_addon_instance__blocking( configured_storage_addon.imp_cls, configured_storage_addon.base_account, configured_storage_addon.config, ) - return async_to_sync(imp.build_wb_config)() + wb_config = async_to_sync(imp.build_wb_config)() + return wb_config