[Integration][Snyk] - Validation to Prevent Fetching Non Existent Use…

…rs (#1098)

# Description

What - Customers had the impression that the integration was bringing
data from other organizations that were not specified during the
installation process. The particular cause of concern was the Snyk API
that enriches the project data with the importer and owner details. It
is possible that an importer or the owner of the Snyk project have left
the organization. In this instance, when we query the API, we will get
404, which is expected. But the fact that the customer sees an API call
to the owners new organization makes it appear that the integration is
pulling data from other org.

Why -

How - Added validation to prevent the integration from making attempts
to fetch users from other organisation instead of the ones provided to
the integration

## Type of change

Please leave one option from the following and delete the rest:

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] New Integration (non-breaking change which adds a new integration)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] Non-breaking change (fix of existing functionality that will not
change current behavior)
- [ ] Documentation (added/updated documentation)

<h4> All tests should be run against the port production
environment(using a testing org). </h4>

### Core testing checklist

- [ ] Integration able to create all default resources from scratch
- [ ] Resync finishes successfully
- [ ] Resync able to create entities
- [ ] Resync able to update entities
- [ ] Resync able to detect and delete entities
- [ ] Scheduled resync able to abort existing resync and start a new one
- [ ] Tested with at least 2 integrations from scratch
- [ ] Tested with Kafka and Polling event listeners
- [ ] Tested deletion of entities that don't pass the selector


### Integration testing checklist

- [ ] Integration able to create all default resources from scratch
- [ ] Resync able to create entities
- [ ] Resync able to update entities
- [ ] Resync able to detect and delete entities
- [ ] Resync finishes successfully
- [ ] If new resource kind is added or updated in the integration, add
example raw data, mapping and expected result to the `examples` folder
in the integration directory.
- [ ] If resource kind is updated, run the integration with the example
data and check if the expected result is achieved
- [ ] If new resource kind is added or updated, validate that
live-events for that resource are working as expected
- [ ] Docs PR link [here](#)

### Preflight checklist

- [ ] Handled rate limiting
- [ ] Handled pagination
- [ ] Implemented the code in async
- [ ] Support Multi account

## Screenshots

Include screenshots from your environment showing how the resources of
the integration will look.

## API Documentation

Provide links to the API documentation used for this integration.

---------

Co-authored-by: shariff-6 <[email protected]>
Co-authored-by: Tom Tankilevitch <[email protected]>

integrations/snyk/CHANGELOG.md

@@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 <!-- towncrier release notes start -->
 
+## 0.1.106 (2024-11-26)
+
+
+### Improvements
+
+- Added validation to skip fetching user details if the user_reference does not belong to an organization associated with the integration, preventing unnecessary API calls for non-existent users
+
+
 ## 0.1.105 (2024-11-25)
 
 

integrations/snyk/pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "snyk"
-version = "0.1.105"
+version = "0.1.106"
 description = "Snyk integration powered by Ocean"
 authors = ["Isaac Coffie <[email protected]>"]
 

integrations/snyk/snyk/client.py

@@ -262,38 +262,23 @@ async def get_single_project(self, org_id: str, project_id: str) -> dict[str, An
 
         return project
 
-    async def _get_user_details_old(self, user_id: str | None) -> dict[str, Any]:
+    async def _get_user_details(self, user_reference: str | None) -> dict[str, Any]:
         if (
-            not user_id
+            not user_reference
         ):  ## Some projects may not have been assigned to any owner yet. In this instance, we can return an empty dict
             return {}
-        cached_details = event.attributes.get(f"{CacheKeys.USER}-{user_id}")
-        if cached_details:
-            return cached_details
-
-        try:
-            user_details = await self._send_api_request(
-                url=f"{self.api_url}/user/{user_id}"
-            )
 
-            if not user_details:
-                return {}
+        # The user_reference is in the format of /rest/orgs/{org_id}/users/{user_id}. Some users may not be associated with the organization that the integration is configured with. In this instance, we can return an empty dict
+        reference_parts = user_reference.split("/")
+        org_id = reference_parts[3]
+        user_id = reference_parts[-1]
 
-            event.attributes[f"{CacheKeys.USER}-{user_id}"] = user_details
-            return user_details
-        except httpx.HTTPStatusError as e:
-            if e.response.status_code == 404:
-                logger.debug(f"user {user_id} not was not found, skipping...")
-                return {}
-            else:
-                raise
-
-    async def _get_user_details(self, user_reference: str | None) -> dict[str, Any]:
-        if (
-            not user_reference
-        ):  ## Some projects may not have been assigned to any owner yet. In this instance, we can return an empty dict
+        if self.organization_ids and org_id not in self.organization_ids:
+            logger.debug(
+                f"User {user_id} in organization {org_id} is not associated with any of the organizations provided to the integration org_id. Skipping..."
+            )
             return {}
-        user_id = user_reference.split("/")[-1]
+
         user_cache_key = f"{CacheKeys.USER}-{user_id}"
         user_reference = user_reference.replace("/rest", "")
         cached_details = event.attributes.get(user_cache_key)

integrations/snyk/tests/snyk/test_snyk_client_user_details.py

@@ -0,0 +1,191 @@
+import pytest
+from unittest.mock import patch, AsyncMock, MagicMock
+import httpx
+from typing import Generator
+from port_ocean.context.event import event
+from port_ocean.context.event import event_context
+from port_ocean.exceptions.context import PortOceanContextAlreadyInitializedError
+from port_ocean.context.ocean import initialize_port_ocean_context
+from snyk.client import SnykClient
+from aiolimiter import AsyncLimiter
+
+MOCK_API_URL = "https://api.test.com"
+MOCK_TOKEN = "test-token"
+MOCK_ORG_URL = "https://test.com"
+MOCK_PERSONAL_ACCESS_TOKEN = "test-personal-token"
+
+
+# Port Ocean Mocks
+@pytest.fixture(autouse=True)
+def mock_ocean_context() -> None:
+    """Fixture to mock the Ocean context initialization."""
+    try:
+        mock_ocean_app = MagicMock()
+        mock_ocean_app.config.integration.config = {
+            "organization_url": MOCK_ORG_URL,
+            "personal_access_token": MOCK_PERSONAL_ACCESS_TOKEN,
+        }
+        mock_ocean_app.integration_router = MagicMock()
+        mock_ocean_app.port_client = MagicMock()
+        initialize_port_ocean_context(mock_ocean_app)
+    except PortOceanContextAlreadyInitializedError:
+        pass
+
+
+class TestSnykClientUserDetails:
+    @pytest.fixture
+    def snyk_client(self) -> SnykClient:
+        """Fixture to create a SnykClient instance for testing."""
+        return SnykClient(
+            token=MOCK_TOKEN,
+            api_url=MOCK_API_URL,
+            app_host=None,
+            organization_ids=["org123"],
+            group_ids=None,
+            webhook_secret=None,
+            rate_limiter=AsyncLimiter(5, 1),
+        )
+
+    @pytest.fixture
+    def mock_event_context(self) -> Generator[MagicMock, None, None]:
+        """Create a mock event context for tests"""
+        mock_event = MagicMock()
+        mock_event.attributes = {}
+
+        with patch("port_ocean.context.event.event", mock_event):
+            yield mock_event
+
+    @pytest.mark.asyncio
+    async def test_none_user_reference(self, snyk_client: SnykClient) -> None:
+        """Test handling of None user reference"""
+        with patch.object(
+            snyk_client, "_send_api_request", new_callable=AsyncMock
+        ) as mock_send_api_request:
+            async with event_context("test_event"):
+                result = await snyk_client._get_user_details(None)
+                assert result == {}
+                mock_send_api_request.assert_not_called()
+
+    @pytest.mark.asyncio
+    async def test_user_from_different_org(self, snyk_client: SnykClient) -> None:
+        """Test handling of user from non-configured organization"""
+        with patch.object(
+            snyk_client, "_send_api_request", new_callable=AsyncMock
+        ) as mock_send_api_request:
+            async with event_context("test_event"):
+                # Arrange
+                user_reference = "/rest/orgs/different_org/users/user123"
+
+                # Act
+                result = await snyk_client._get_user_details(user_reference)
+
+                # Assert
+                assert result == {}
+                mock_send_api_request.assert_not_called()
+
+    @pytest.mark.asyncio
+    async def test_cached_user_details(self, snyk_client: SnykClient) -> None:
+        """Test retrieval of cached user details"""
+        with patch.object(
+            snyk_client, "_send_api_request", new_callable=AsyncMock
+        ) as mock_send_api_request:
+            async with event_context("test_event"):
+                # Arrange
+                user_id = "user123"
+                user_reference = f"/rest/orgs/org123/users/{user_id}"
+                cached_data = {"data": {"id": user_id, "name": "Test User"}}
+                event.attributes[f"user-{user_id}"] = cached_data
+
+                # Act
+                result = await snyk_client._get_user_details(user_reference)
+
+                # Assert
+                assert result == cached_data
+                mock_send_api_request.assert_not_called()
+
+    @pytest.mark.asyncio
+    async def test_successful_user_details_fetch(self, snyk_client: SnykClient) -> None:
+        """Test successful user details fetch"""
+        with patch.object(
+            snyk_client, "_send_api_request", new_callable=AsyncMock
+        ) as mock_send_api_request:
+            async with event_context("test_event"):
+                # Arrange
+                user_id = "user123"
+                user_reference = f"/rest/orgs/org123/users/{user_id}"
+                api_response = {"data": {"id": user_id, "name": "Test User"}}
+                mock_send_api_request.return_value = api_response
+
+                # Act
+                result = await snyk_client._get_user_details(user_reference)
+
+                # Assert
+                assert result == api_response["data"]
+                mock_send_api_request.assert_called_once_with(
+                    url=f"{MOCK_API_URL}/rest/orgs/org123/users/{user_id}",
+                    query_params={"version": "2024-06-21~beta"},
+                )
+                assert event.attributes[f"user-{user_id}"] == api_response
+
+    @pytest.mark.asyncio
+    async def test_404_error_handling(self, snyk_client: SnykClient) -> None:
+        """Test 404 error handling"""
+        with patch.object(
+            snyk_client, "_send_api_request", new_callable=AsyncMock
+        ) as mock_send_api_request:
+            async with event_context("test_event"):
+                # Arrange
+                user_reference = "/rest/orgs/org123/users/user123"
+                mock_response = MagicMock(spec=httpx.Response)
+                mock_response.status_code = 404
+                error = httpx.HTTPStatusError(
+                    "Not Found",
+                    request=MagicMock(spec=httpx.Request),
+                    response=mock_response,
+                )
+                mock_send_api_request.side_effect = error
+
+                # Act
+                result = await snyk_client._get_user_details(user_reference)
+
+                # Assert
+                assert result == {}
+
+    @pytest.mark.asyncio
+    async def test_non_404_error_handling(self, snyk_client: SnykClient) -> None:
+        """Test non-404 error handling"""
+        with patch.object(
+            snyk_client, "_send_api_request", new_callable=AsyncMock
+        ) as mock_send_api_request:
+            async with event_context("test_event"):
+                # Arrange
+                user_reference = "/rest/orgs/org123/users/user123"
+                mock_response = MagicMock(spec=httpx.Response)
+                mock_response.status_code = 500
+                error = httpx.HTTPStatusError(
+                    "Server Error",
+                    request=MagicMock(spec=httpx.Request),
+                    response=mock_response,
+                )
+                mock_send_api_request.side_effect = error
+
+                # Act/Assert
+                with pytest.raises(httpx.HTTPStatusError):
+                    await snyk_client._get_user_details(user_reference)
+
+    @pytest.mark.asyncio
+    async def test_empty_api_response(self, snyk_client: SnykClient) -> None:
+        """Test empty API response"""
+        with patch.object(
+            snyk_client, "_send_api_request", new_callable=AsyncMock
+        ) as mock_send_api_request:
+            async with event_context("test_event"):
+                # Arrange
+                user_reference = "/rest/orgs/org123/users/user123"
+                mock_send_api_request.return_value = None
+
+                # Act
+                result = await snyk_client._get_user_details(user_reference)
+
+                # Assert
+                assert result == {}