diff --git a/integrations/snyk/CHANGELOG.md b/integrations/snyk/CHANGELOG.md index 1c739b3e6f..1e65b9dfba 100644 --- a/integrations/snyk/CHANGELOG.md +++ b/integrations/snyk/CHANGELOG.md @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 +## 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) diff --git a/integrations/snyk/pyproject.toml b/integrations/snyk/pyproject.toml index 719b1cac9e..c5c14f9cea 100644 --- a/integrations/snyk/pyproject.toml +++ b/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 "] diff --git a/integrations/snyk/snyk/client.py b/integrations/snyk/snyk/client.py index e86af6681f..930302176b 100644 --- a/integrations/snyk/snyk/client.py +++ b/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) diff --git a/integrations/snyk/tests/snyk/test_snyk_client_user_details.py b/integrations/snyk/tests/snyk/test_snyk_client_user_details.py new file mode 100644 index 0000000000..8db3f95dc0 --- /dev/null +++ b/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 == {}