diff --git a/src/solace_agent_mesh/common/services/default_resource_sharing_service.py b/src/solace_agent_mesh/common/services/default_resource_sharing_service.py index ae06e7daf1..3cfe9c6cf8 100644 --- a/src/solace_agent_mesh/common/services/default_resource_sharing_service.py +++ b/src/solace_agent_mesh/common/services/default_resource_sharing_service.py @@ -31,4 +31,23 @@ def delete_resource_shares( resource_type: ResourceType ) -> bool: """Community has no shares to delete - return True (no-op success).""" - return True \ No newline at end of file + return True + + def unshare_users_from_resource( + self, + session, + resource_id: str, + resource_type: ResourceType, + user_emails: List[str] + ) -> bool: + """Community has no shares to unshare - return True (no-op success).""" + return True + + def get_shared_users( + self, + session, + resource_id: str, + resource_type: ResourceType + ) -> List[str]: + """Community has no sharing - return empty list.""" + return [] diff --git a/src/solace_agent_mesh/gateway/http_sse/routers/tasks.py b/src/solace_agent_mesh/gateway/http_sse/routers/tasks.py index f2284cf9c9..55d9bf90c1 100644 --- a/src/solace_agent_mesh/gateway/http_sse/routers/tasks.py +++ b/src/solace_agent_mesh/gateway/http_sse/routers/tasks.py @@ -55,6 +55,8 @@ log = logging.getLogger(__name__) +SESSION_NOT_FOUND_MSG = "Session not found." + # Background Task Status Models and Endpoints class TaskStatusResponse(BaseModel): @@ -385,6 +387,36 @@ async def _submit_task( finally: db.close() + # Security: Validate user still has project access + if project_id and project_service: + if SessionLocal is not None: + db = SessionLocal() + try: + project = project_service.get_project(db, project_id, user_id) + if not project: + log.warning( + "%sUser %s denied - project %s not found or access denied", + log_prefix, + user_id, + project_id + ) + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail=SESSION_NOT_FOUND_MSG + ) + except HTTPException: + raise + except Exception as e: + log.error( + "%sFailed to validate project access: %s", log_prefix, e + ) + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail=SESSION_NOT_FOUND_MSG + ) + finally: + db.close() + if frontend_session_id: session_id = frontend_session_id log.info( @@ -555,6 +587,9 @@ async def _submit_task( result=task_object, request_id=payload.id ) + except HTTPException: + # Re-raise HTTPExceptions (including our security check) without wrapping + raise except PermissionError as pe: log.warning("%sPermission denied: %s", log_prefix, str(pe)) raise HTTPException( diff --git a/src/solace_agent_mesh/gateway/http_sse/services/project_service.py b/src/solace_agent_mesh/gateway/http_sse/services/project_service.py index 3fa465789b..aa2b16827c 100644 --- a/src/solace_agent_mesh/gateway/http_sse/services/project_service.py +++ b/src/solace_agent_mesh/gateway/http_sse/services/project_service.py @@ -693,11 +693,21 @@ def soft_delete_project(self, db, project_id: str, user_id: str) -> bool: if not soft_deleted: return False - # Cascade to sessions from ..repository.session_repository import SessionRepository session_repo = SessionRepository() - deleted_count = session_repo.soft_delete_by_project(db, project_id, user_id) - self.logger.info(f"Successfully soft deleted project {project_id} and {deleted_count} associated sessions") + + owner_deleted_count = session_repo.soft_delete_by_project(db, project_id, user_id) + + self._resource_sharing_service.delete_resource_shares( + session=db, + resource_id=project_id, + resource_type=ResourceType.PROJECT + ) + + self.logger.info( + f"Successfully soft deleted project {project_id} and {owner_deleted_count} owner sessions " + f"(shared users handled by sharing service)" + ) return True diff --git a/src/solace_agent_mesh/gateway/http_sse/services/session_service.py b/src/solace_agent_mesh/gateway/http_sse/services/session_service.py index f9ee66cfb3..9f02403a8b 100644 --- a/src/solace_agent_mesh/gateway/http_sse/services/session_service.py +++ b/src/solace_agent_mesh/gateway/http_sse/services/session_service.py @@ -313,12 +313,9 @@ async def move_session_to_project( # Validate project exists and user has access if project_id is provided if new_project_id: - from ..repository.models import ProjectModel - project = db.query(ProjectModel).filter( - ProjectModel.id == new_project_id, - ProjectModel.user_id == user_id, - ProjectModel.deleted_at.is_(None) - ).first() + from .project_service import ProjectService + project_service = ProjectService(component=self.component) + project = project_service.get_project(db, new_project_id, user_id) if not project: raise ValueError(f"Project {new_project_id} not found or access denied") diff --git a/src/solace_agent_mesh/services/resource_sharing_service.py b/src/solace_agent_mesh/services/resource_sharing_service.py index 7366a96bb8..d3889e440c 100644 --- a/src/solace_agent_mesh/services/resource_sharing_service.py +++ b/src/solace_agent_mesh/services/resource_sharing_service.py @@ -56,7 +56,53 @@ def delete_resource_shares( """ Delete all sharing records for a resource (e.g., when resource is deleted). + IMPORTANT: Implementations MUST also cleanup any dependent data + (e.g., sessions created by shared users) to maintain data consistency. + Returns: True if successful, False otherwise. """ + pass + + @abstractmethod + def unshare_users_from_resource( + self, + session, + resource_id: str, + resource_type: ResourceType, + user_emails: List[str] + ) -> bool: + """ + Remove specific users' access to a resource. + + IMPORTANT: Implementations MUST cleanup dependent data + (e.g., sessions) when removing access. + + Args: + session: Database session + resource_id: The resource ID + resource_type: Type of resource (e.g., PROJECT) + user_emails: List of user emails to unshare + + Returns: + True if successful, False otherwise. + """ + pass + + @abstractmethod + def get_shared_users( + self, + session, + resource_id: str, + resource_type: ResourceType + ) -> List[str]: + """ + Get list of user emails with shared access to a resource. + + This is used when deleting resources to ensure sessions created by + shared users are also cleaned up. + + Returns: + List of user emails. Empty list for community, actual emails for enterprise. + """ pass \ No newline at end of file diff --git a/tests/integration/apis/persistence/test_tasks_api.py b/tests/integration/apis/persistence/test_tasks_api.py index c37a8760b6..04d4e95b02 100644 --- a/tests/integration/apis/persistence/test_tasks_api.py +++ b/tests/integration/apis/persistence/test_tasks_api.py @@ -780,3 +780,155 @@ def test_task_and_session_integration(api_client: TestClient): assert user_message["message"] == "Integration test message" print(f"✓ Task-session integration verified for session {session_id}") + + +def test_send_message_to_nonexistent_project_returns_404(api_client: TestClient): + """Test that sending a message with non-existent project_id returns 404""" + import uuid + + nonexistent_project_id = str(uuid.uuid4()) + + task_payload = { + "jsonrpc": "2.0", + "id": str(uuid.uuid4()), + "method": "message/stream", + "params": { + "message": { + "role": "user", + "messageId": str(uuid.uuid4()), + "kind": "message", + "parts": [{"kind": "text", "text": "Test message"}], + "metadata": { + "agent_name": "TestAgent", + "project_id": nonexistent_project_id, + }, + } + }, + } + + response = api_client.post("/api/v1/message:stream", json=task_payload) + + # Should return 404 when project doesn't exist + assert response.status_code == 404 + response_data = response.json() + # HTTPException returns 'detail' field directly, or wrapped in 'message' by error handler + error_message = response_data.get("detail") or response_data.get("message", "") + assert "Session not found" in error_message + + print("Non-existent project returns 404 with standard message") + + +def test_send_message_to_valid_project(api_client: TestClient, gateway_adapter): + """Test that sending a message to a valid project succeeds""" + import uuid + + # First create a project using gateway adapter + project_id = str(uuid.uuid4()) + gateway_adapter.seed_project( + project_id=project_id, + name="Test Project for Message", + user_id="sam_dev_user", + description="Project for testing message submission", + ) + + # Now send a message with that project_id + task_payload = { + "jsonrpc": "2.0", + "id": str(uuid.uuid4()), + "method": "message/stream", + "params": { + "message": { + "role": "user", + "messageId": str(uuid.uuid4()), + "kind": "message", + "parts": [{"kind": "text", "text": "Message to valid project"}], + "metadata": { + "agent_name": "TestAgent", + "project_id": project_id, + }, + } + }, + } + + response = api_client.post("/api/v1/message:stream", json=task_payload) + + # Should succeed + assert response.status_code == 200 + response_data = response.json() + assert "result" in response_data + assert "id" in response_data["result"] + + print(f"Message to valid project {project_id} succeeded") + + +def test_send_message_without_project_id_succeeds(api_client: TestClient): + """Test that messages without project_id work normally (non-project sessions)""" + import uuid + + task_payload = { + "jsonrpc": "2.0", + "id": str(uuid.uuid4()), + "method": "message/stream", + "params": { + "message": { + "role": "user", + "messageId": str(uuid.uuid4()), + "kind": "message", + "parts": [{"kind": "text", "text": "Non-project message"}], + "metadata": { + "agent_name": "TestAgent", + # No project_id - regular session + }, + } + }, + } + + response = api_client.post("/api/v1/message:stream", json=task_payload) + + # Should succeed without project access check + assert response.status_code == 200 + response_data = response.json() + assert "result" in response_data + + print("Message without project_id succeeded (regular session)") + + +def test_send_message_project_access_check_handles_exceptions(api_client: TestClient): + """Test that project access validation handles database errors gracefully""" + import uuid + from unittest.mock import patch + + # Create a valid project + project_id = str(uuid.uuid4()) + + task_payload = { + "jsonrpc": "2.0", + "id": str(uuid.uuid4()), + "method": "message/stream", + "params": { + "message": { + "role": "user", + "messageId": str(uuid.uuid4()), + "kind": "message", + "parts": [{"kind": "text", "text": "Test message"}], + "metadata": { + "agent_name": "TestAgent", + "project_id": project_id, + }, + } + }, + } + + # Mock project_service.get_project to raise an exception + with patch('solace_agent_mesh.gateway.http_sse.services.project_service.ProjectService.get_project') as mock_get: + mock_get.side_effect = Exception("Database error") + + response = api_client.post("/api/v1/message:stream", json=task_payload) + + # Should return 404 due to exception in access validation + assert response.status_code == 404 + response_data = response.json() + error_message = response_data.get("detail") or response_data.get("message", "") + assert "Session not found" in error_message + + print("Exception in project access check handled correctly") diff --git a/tests/unit/services/test_default_resource_sharing_service.py b/tests/unit/services/test_default_resource_sharing_service.py index e15a8f0bf0..1f5fcf1ec1 100644 --- a/tests/unit/services/test_default_resource_sharing_service.py +++ b/tests/unit/services/test_default_resource_sharing_service.py @@ -62,3 +62,52 @@ def test_delete_resource_shares_returns_true(self): ) assert result is True + + def test_unshare_users_from_resource_returns_true(self): + """Test that unshare_users_from_resource returns True indicating successful operation. + + When users are unshared from a resource, this method is called to remove + their access. In community edition, there are no shares to remove, but + the operation succeeds (no-op). Returning True indicates the operation + completed successfully without errors. + """ + user_emails = ["user1@example.com", "user2@example.com"] + + result = self.service.unshare_users_from_resource( + session=self.mock_session, + resource_id=self.resource_id, + resource_type=self.resource_type, + user_emails=user_emails, + ) + + assert result is True + + def test_unshare_users_from_resource_handles_empty_list(self): + """Test that unshare_users_from_resource handles empty email list correctly. + + Edge case: when called with an empty list of user emails, the method + should still succeed (no-op on empty input). + """ + result = self.service.unshare_users_from_resource( + session=self.mock_session, + resource_id=self.resource_id, + resource_type=self.resource_type, + user_emails=[], + ) + + assert result is True + + def test_get_shared_users_returns_empty_list(self): + """Test that get_shared_users returns an empty list. + + This method is used to retrieve users who have shared access to a resource. + In community edition, no users have shared access (only ownership exists), + so this method always returns an empty list. + """ + result = self.service.get_shared_users( + session=self.mock_session, + resource_id=self.resource_id, + resource_type=self.resource_type, + ) + + assert result == [] diff --git a/tests/unit/services/test_resource_sharing_service_abc.py b/tests/unit/services/test_resource_sharing_service_abc.py new file mode 100644 index 0000000000..ee7d0d7563 --- /dev/null +++ b/tests/unit/services/test_resource_sharing_service_abc.py @@ -0,0 +1,49 @@ +"""Tests for ResourceSharingService abstract base class.""" + +from solace_agent_mesh.services.resource_sharing_service import ( + ResourceSharingService, + ResourceType, +) + + +class ConcreteResourceSharingService(ResourceSharingService): + """Concrete implementation for testing the ABC.""" + + def get_shared_resource_ids(self, session, user_email: str, resource_type: ResourceType): + return [] + + def check_user_access(self, session, resource_id: str, resource_type: ResourceType, user_email: str): + return None + + def delete_resource_shares(self, session, resource_id: str, resource_type: ResourceType): + return True + + def unshare_users_from_resource(self, session, resource_id: str, resource_type: ResourceType, user_emails): + return True + + def get_shared_users(self, session, resource_id: str, resource_type: ResourceType): + return [] + + +class TestResourceSharingServiceABC: + """Test the abstract base class contract.""" + + def test_resource_type_enum_has_project(self): + """Test that ResourceType enum includes PROJECT.""" + assert ResourceType.PROJECT.value == "project" + + def test_is_resource_sharing_available_default_false(self): + """Test that default is_resource_sharing_available returns False.""" + service = ConcreteResourceSharingService() + assert service.is_resource_sharing_available is False + + def test_abstract_methods_can_be_implemented(self): + """Test that all abstract methods can be implemented.""" + service = ConcreteResourceSharingService() + + # Test all abstract methods can be called + assert service.get_shared_resource_ids(None, "test@example.com", ResourceType.PROJECT) == [] + assert service.check_user_access(None, "id", ResourceType.PROJECT, "test@example.com") is None + assert service.delete_resource_shares(None, "id", ResourceType.PROJECT) is True + assert service.unshare_users_from_resource(None, "id", ResourceType.PROJECT, []) is True + assert service.get_shared_users(None, "id", ResourceType.PROJECT) == [] diff --git a/tests/unit/services/test_session_service_move_to_project.py b/tests/unit/services/test_session_service_move_to_project.py new file mode 100644 index 0000000000..f1b369891b --- /dev/null +++ b/tests/unit/services/test_session_service_move_to_project.py @@ -0,0 +1,139 @@ +"""Unit tests for SessionService.move_session_to_project method.""" + +import pytest +from unittest.mock import Mock, MagicMock, patch +from solace_agent_mesh.gateway.http_sse.services.session_service import SessionService + + +class TestSessionServiceMoveToProject: + """Test the move_session_to_project method.""" + + def setup_method(self): + """Set up test fixtures.""" + self.mock_component = Mock() + self.service = SessionService(component=self.mock_component) + self.mock_db = Mock() + self.session_id = "test-session-123" + self.user_id = "user-123" + self.project_id = "project-456" + + @pytest.mark.asyncio + async def test_move_session_validates_project_access_for_owner(self): + """Test that moving session validates user has access to target project (owner).""" + # Mock the project service to return a valid project (user is owner) + with patch('solace_agent_mesh.gateway.http_sse.services.project_service.ProjectService') as MockProjectService: + mock_project_service = Mock() + mock_project = Mock() + mock_project.id = self.project_id + mock_project_service.get_project.return_value = mock_project + MockProjectService.return_value = mock_project_service + + # Mock session repository + mock_repository = Mock() + mock_updated_session = Mock() + mock_updated_session.id = self.session_id + mock_updated_session.project_id = self.project_id + mock_repository.move_to_project.return_value = mock_updated_session + + with patch.object(self.service, '_get_repositories', return_value=mock_repository): + # Execute + result = await self.service.move_session_to_project( + db=self.mock_db, + session_id=self.session_id, + user_id=self.user_id, + new_project_id=self.project_id + ) + + # Assert project access was checked + mock_project_service.get_project.assert_called_once_with( + self.mock_db, self.project_id, self.user_id + ) + + # Assert session was moved + assert result is not None + assert result.project_id == self.project_id + + @pytest.mark.asyncio + async def test_move_session_validates_project_access_for_shared_viewer(self): + """Test that moving session validates user has access to target project (shared viewer).""" + # Mock the project service to return a valid project (user has viewer access via sharing) + with patch('solace_agent_mesh.gateway.http_sse.services.project_service.ProjectService') as MockProjectService: + mock_project_service = Mock() + mock_project = Mock() + mock_project.id = self.project_id + mock_project_service.get_project.return_value = mock_project # Has access via sharing + MockProjectService.return_value = mock_project_service + + # Mock session repository + mock_repository = Mock() + mock_updated_session = Mock() + mock_updated_session.id = self.session_id + mock_updated_session.project_id = self.project_id + mock_repository.move_to_project.return_value = mock_updated_session + + with patch.object(self.service, '_get_repositories', return_value=mock_repository): + # Execute + result = await self.service.move_session_to_project( + db=self.mock_db, + session_id=self.session_id, + user_id=self.user_id, + new_project_id=self.project_id + ) + + # Assert project access was checked + mock_project_service.get_project.assert_called_once() + + # Assert session was moved (user has viewer access) + assert result is not None + assert result.project_id == self.project_id + + @pytest.mark.asyncio + async def test_move_session_rejects_nonexistent_project(self): + """Test that moving session to non-existent project raises ValueError.""" + # Mock the project service to return None (project doesn't exist or no access) + with patch('solace_agent_mesh.gateway.http_sse.services.project_service.ProjectService') as MockProjectService: + mock_project_service = Mock() + mock_project_service.get_project.return_value = None # No access or doesn't exist + MockProjectService.return_value = mock_project_service + + # Execute and expect ValueError + with pytest.raises(ValueError, match="not found or access denied"): + await self.service.move_session_to_project( + db=self.mock_db, + session_id=self.session_id, + user_id=self.user_id, + new_project_id=self.project_id + ) + + # Assert project access was checked + mock_project_service.get_project.assert_called_once_with( + self.mock_db, self.project_id, self.user_id + ) + + @pytest.mark.asyncio + async def test_move_session_allows_removing_from_project(self): + """Test that moving session with None project_id removes it from project.""" + # Mock session repository + mock_repository = Mock() + mock_updated_session = Mock() + mock_updated_session.id = self.session_id + mock_updated_session.project_id = None + mock_repository.move_to_project.return_value = mock_updated_session + + with patch.object(self.service, '_get_repositories', return_value=mock_repository): + # Execute with None project_id + result = await self.service.move_session_to_project( + db=self.mock_db, + session_id=self.session_id, + user_id=self.user_id, + new_project_id=None # Remove from project + ) + + # Assert session was moved (project_id = None) + assert result is not None + assert result.project_id is None + + # Project access should NOT be checked when removing from project + mock_repository.move_to_project.assert_called_once_with( + self.mock_db, self.session_id, self.user_id, None + )