fix: add precheck method to SharePoint connector (#556)

CyMule · web-flow · commit d9bcf3543000 · 2025-07-15T12:43:44.000-04:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,8 @@
+## 1.0.55
+
+* **Fix: add precheck method to SharePoint connector**
+
+
 ## 1.0.54
 
 * **Fix bump Togetherai dependency**
diff --git a/test/integration/connectors/test_sharepoint.py b/test/integration/connectors/test_sharepoint.py
@@ -326,3 +326,120 @@ async def test_sharepoint_library_with_path(temp_dir):
             postdownload_file_data_check=source_filedata_display_name_set_check,
         ),
     )
+
+
+@pytest.fixture
+def base_sharepoint_config():
+    """Base SharePoint config for testing."""
+    config = sharepoint_config()
+    return {
+        'client_id': config.client_id,
+        'tenant': config.tenant,
+        'site': "https://unstructuredio.sharepoint.com/sites/utic-platform-test-source",
+        'client_cred': config.client_cred
+    }
+
+
+@pytest.fixture
+def indexer_factory(base_sharepoint_config):
+    """Factory for creating SharePoint indexers with different configs."""
+    def _create_indexer(client_cred=None, site=None, path=None):
+        access_config = SharepointAccessConfig(
+            client_cred=client_cred or base_sharepoint_config['client_cred']
+        )
+        connection_config = SharepointConnectionConfig(
+            client_id=base_sharepoint_config['client_id'],
+            site=site or base_sharepoint_config['site'],
+            tenant=base_sharepoint_config['tenant'],
+            access_config=access_config,
+        )
+        index_config = SharepointIndexerConfig(path=path or "")
+        return SharepointIndexer(
+            connection_config=connection_config,
+            index_config=index_config,
+        )
+    return _create_indexer
+
+
+@pytest.mark.asyncio
+@pytest.mark.tags(CONNECTOR_TYPE, SOURCE_TAG, BLOB_STORAGE_TAG)
+@requires_env("SHAREPOINT_CLIENT_ID", "SHAREPOINT_CRED", "MS_TENANT_ID", "MS_USER_PNAME")
+@pytest.mark.parametrize("error_scenario,expected_error", [
+    ("invalid_creds", "UserAuthError"),
+    ("nonexistent_site", "UserError"),
+    ("invalid_path", "UserError"),
+])
+async def test_sharepoint_precheck_error_scenarios(indexer_factory, error_scenario, expected_error):
+    """Parametrized test for different SharePoint precheck error scenarios."""
+    from unstructured_ingest.errors_v2 import UserAuthError, UserError
+    
+    error_class_map = {
+        "UserAuthError": UserAuthError,
+        "UserError": UserError
+    }
+    
+    expected_exception = error_class_map[expected_error]
+    
+    if error_scenario == "invalid_creds":
+        indexer = indexer_factory(client_cred="invalid_creds")
+    elif error_scenario == "nonexistent_site":
+        indexer = indexer_factory(site="https://unstructuredai.sharepoint.com/sites/definitely-does-not-exist-12345")
+    elif error_scenario == "invalid_path":
+        indexer = indexer_factory(path="NonExistentFolder/SubFolder/DoesNotExist")
+    else:
+        pytest.fail(f"Unknown error scenario: {error_scenario}")
+    
+    with pytest.raises(expected_exception):
+        indexer.precheck()
+
+
+@pytest.fixture
+def insufficient_perms_config():
+    """Config for testing insufficient permissions."""
+    from types import SimpleNamespace
+    
+    required_vars = [
+        "SHAREPOINT_CLIENT_ID_INSUFFICIENT",
+        "SHAREPOINT_CRED_INSUFFICIENT", 
+        "MS_TENANT_ID_INSUFFICIENT"
+    ]
+    
+    missing_vars = [var for var in required_vars if var not in os.environ]
+    if missing_vars:
+        pytest.skip(f"Missing environment variables: {missing_vars}")
+    
+    return SimpleNamespace(
+        client_id=os.environ["SHAREPOINT_CLIENT_ID_INSUFFICIENT"],
+        client_cred=os.environ["SHAREPOINT_CRED_INSUFFICIENT"],
+        tenant=os.environ["MS_TENANT_ID_INSUFFICIENT"]
+    )
+
+
+@pytest.mark.asyncio
+@pytest.mark.tags(CONNECTOR_TYPE, SOURCE_TAG, BLOB_STORAGE_TAG)
+@requires_env("MS_USER_PNAME")
+async def test_sharepoint_precheck_insufficient_permissions(
+    insufficient_perms_config,
+    base_sharepoint_config,
+):
+    """Test precheck with credentials that have insufficient permissions."""
+    from unstructured_ingest.errors_v2 import UserAuthError
+
+    access_config = SharepointAccessConfig(client_cred=insufficient_perms_config.client_cred)
+    connection_config = SharepointConnectionConfig(
+        client_id=insufficient_perms_config.client_id,
+        site=base_sharepoint_config['site'],
+        tenant=insufficient_perms_config.tenant,
+        access_config=access_config,
+    )
+    index_config = SharepointIndexerConfig()
+
+    indexer = SharepointIndexer(
+        connection_config=connection_config,
+        index_config=index_config,
+    )
+
+    with pytest.raises(UserAuthError):
+        indexer.precheck()
+
+
diff --git a/unstructured_ingest/__version__.py b/unstructured_ingest/__version__.py
@@ -1 +1 @@
-__version__ = "1.0.54"  # pragma: no cover
+__version__ = "1.0.55"  # pragma: no cover
diff --git a/unstructured_ingest/processes/connectors/onedrive.py b/unstructured_ingest/processes/connectors/onedrive.py
@@ -20,6 +20,7 @@
     SourceConnectionError,
     SourceConnectionNetworkError,
 )
+from unstructured_ingest.errors_v2 import UserAuthError
 from unstructured_ingest.interfaces import (
     AccessConfig,
     ConnectionConfig,
@@ -114,12 +115,27 @@ def get_token(self):
             except ValueError as exc:
                 logger.error("Couldn't set up credentials.")
                 raise exc
+            
             if "error" in token:
-                raise SourceConnectionNetworkError(
-                    "failed to fetch token, {}: {}".format(
-                        token["error"], token["error_description"]
+                error_codes = token.get("error_codes", [])
+                error_type = token.get("error", "")
+                error_description = token.get("error_description", "")
+                
+                # 7000215: Invalid client secret provided
+                # 7000218: Invalid client id provided
+                # 700016: Application not found in directory
+                # 90002: Tenant not found
+                auth_error_codes = [7000215, 7000218, 700016, 90002]
+                
+                if (any(code in error_codes for code in auth_error_codes) or 
+                    error_type in ["invalid_client", "unauthorized_client", "invalid_grant"]):
+                    raise UserAuthError(
+                        f"Authentication failed: {error_type}: {error_description}"
+                    )
+                else:
+                    raise SourceConnectionNetworkError(
+                        f"Failed to fetch token: {error_type}: {error_description}"
                     )
-                )
             return token
 
     @requires_dependencies(["office365"], extras="onedrive")
diff --git a/unstructured_ingest/processes/connectors/sharepoint.py b/unstructured_ingest/processes/connectors/sharepoint.py
@@ -13,6 +13,7 @@
     SourceConnectionError,
     SourceConnectionNetworkError,
 )
+from unstructured_ingest.errors_v2 import UserAuthError, UserError
 from unstructured_ingest.logger import logger
 from unstructured_ingest.processes.connector_registry import (
     SourceRegistryEntry,
@@ -30,6 +31,7 @@
 if TYPE_CHECKING:
     from office365.onedrive.driveitems.driveItem import DriveItem
     from office365.onedrive.sites.site import Site
+    from office365.runtime.client_request_exception import ClientRequestException
 
 CONNECTOR_TYPE = "sharepoint"
 LEGACY_DEFAULT_PATH = "Shared Documents"
@@ -82,15 +84,94 @@ def _get_drive_item(self, client_site: Site) -> DriveItem:
 
 
 class SharepointIndexerConfig(OnedriveIndexerConfig):
-    pass
-
+    # TODO: We can probably make path non-optional on OnedriveIndexerConfig once tested
+    path: str = Field(default="")
 
 @dataclass
 class SharepointIndexer(OnedriveIndexer):
     connection_config: SharepointConnectionConfig
     index_config: SharepointIndexerConfig
     connector_type: str = CONNECTOR_TYPE
 
+    def _handle_client_request_exception(self, e: ClientRequestException, context: str) -> None:
+        """Convert ClientRequestException to appropriate user-facing error based on HTTP status."""
+        if hasattr(e, "response") and e.response is not None and hasattr(e.response, "status_code"):
+            status_code = e.response.status_code
+            if status_code == 401:
+                raise UserAuthError(
+                    f"Unauthorized access to {context}. Check client credentials and permissions"
+                )
+            elif status_code == 403:
+                raise UserAuthError(
+                    f"Access forbidden to {context}. "
+                    f"Check app permissions (Sites.Read.All required)"
+                )
+            elif status_code == 404:
+                raise UserError(f"Not found: {context}")
+
+        raise UserError(f"Failed to access {context}: {str(e)}")
+
+    def _is_root_path(self, path: str) -> bool:
+        """Check if the path represents root access (empty string or legacy default)."""
+        return not path or not path.strip() or path == LEGACY_DEFAULT_PATH
+    
+    def _get_target_drive_item(self, site_drive_item: DriveItem, path: str) -> DriveItem:
+        """Get the drive item to search in based on the path."""
+        if self._is_root_path(path):
+            return site_drive_item
+        else:
+            return site_drive_item.get_by_path(path).get().execute_query()
+    
+    def _validate_folder_path(self, site_drive_item: DriveItem, path: str) -> None:
+        """Validate that a specific folder path exists and is accessible."""
+        from office365.runtime.client_request_exception import ClientRequestException
+
+        try:
+            path_item = site_drive_item.get_by_path(path).get().execute_query()
+            if path_item is None or not hasattr(path_item, "is_folder"):
+                raise UserError(
+                    f"SharePoint path '{path}' not found in site {self.connection_config.site}. "
+                    f"Check that the path exists and you have access to it"
+                )
+            logger.info(f"SharePoint folder path '{path}' validated successfully")
+        except ClientRequestException as e:
+            logger.error(f"Failed to access SharePoint path '{path}': {e}")
+            self._handle_client_request_exception(e, f"SharePoint path '{path}'")
+        except Exception as e:
+            logger.error(f"Unexpected error accessing SharePoint path '{path}': {e}")
+            raise UserError(f"Failed to validate SharePoint path '{path}': {str(e)}")
+
+    @requires_dependencies(["office365"], extras="sharepoint")
+    def precheck(self) -> None:
+        """Validate SharePoint connection before indexing."""
+        from office365.runtime.client_request_exception import ClientRequestException
+
+        # Validate authentication - this call will raise UserAuthError if invalid
+        self.connection_config.get_token()
+
+        try:
+            client = self.connection_config.get_client()
+            client_site = client.sites.get_by_url(self.connection_config.site).get().execute_query()
+            site_drive_item = self.connection_config._get_drive_item(client_site)
+
+            path = self.index_config.path
+            if not self._is_root_path(path):
+                self._validate_folder_path(site_drive_item, path)
+
+            logger.info(
+                f"SharePoint connection validated successfully for site: "
+                f"{self.connection_config.site}"
+            )
+
+        except ClientRequestException as e:
+            logger.error(f"SharePoint precheck failed for site: {self.connection_config.site}")
+            self._handle_client_request_exception(
+                e, f"SharePoint site {self.connection_config.site}"
+            )
+        except Exception as e:
+            logger.error(f"Unexpected error during SharePoint precheck: {e}", exc_info=True)
+            raise UserError(f"Failed to validate SharePoint connection: {str(e)}")
+
     @requires_dependencies(["office365"], extras="sharepoint")
     async def run_async(self, **kwargs: Any) -> AsyncIterator[FileData]:
         from office365.runtime.client_request_exception import ClientRequestException
@@ -113,11 +194,11 @@ async def run_async(self, **kwargs: Any) -> AsyncIterator[FileData]:
             )
 
         path = self.index_config.path
-        # Deprecated sharepoint sdk needed a default path. Microsoft Graph SDK does not.
-        if path and path != LEGACY_DEFAULT_PATH:
-            site_drive_item = site_drive_item.get_by_path(path).get().execute_query()
+        target_drive_item = await asyncio.to_thread(
+            self._get_target_drive_item, site_drive_item, path
+        )
 
-        for drive_item in site_drive_item.get_files(
+        for drive_item in target_drive_item.get_files(
             recursive=self.index_config.recursive
         ).execute_query():
             file_data = await self.drive_item_to_file_data(drive_item=drive_item)

Original file line number	Diff line number	Diff line change
`@@ -1 +1 @@`
`1`		`-__version__ = "1.0.54" # pragma: no cover`
	`1`	`+__version__ = "1.0.55" # pragma: no cover`