[ENG-6569] | wb_config fix for Owncloud and Bitbucket + Owncloud Impl…

…ementation Connection Fix bug (#167)

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}"
-            )

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:

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)

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