Add webhook endpoint authentication bypass and admin context unfiltered data access (#12956)

Co-authored-by: openhands <openhands@all-hands.dev>

enterprise/server/middleware.py

@@ -164,7 +164,6 @@ class SetAuthCookieMiddleware:
             '/oauth/device/authorize',
             '/oauth/device/token',
             '/api/v1/web-client/config',
-            '/api/v1/webhooks/secrets',
         )
         if path in ignore_paths:
             return False
@@ -175,6 +174,10 @@ class SetAuthCookieMiddleware:
         ):
             return False
 
+        # Webhooks access is controlled using separate API keys
+        if path.startswith('/api/v1/webhooks/'):
+            return False
+
         is_mcp = path.startswith('/mcp')
         is_api_route = path.startswith('/api')
         return is_api_route or is_mcp

enterprise/server/utils/saas_app_conversation_info_injector.py

@@ -24,6 +24,7 @@ from openhands.app_server.app_conversation.sql_app_conversation_info_service imp
 )
 from openhands.app_server.errors import AuthError
 from openhands.app_server.services.injector import InjectorState
+from openhands.app_server.user.specifiy_user_context import ADMIN
 
 
 class SaasSQLAppConversationInfoService(SQLAppConversationInfoService):
@@ -63,6 +64,12 @@ class SaasSQLAppConversationInfoService(SQLAppConversationInfoService):
         Raises:
             AuthError: If no user_id is available (secure default: deny access)
         """
+        # For internal operations such as getting a conversation by session_api_key
+        # we need a mode that does not have filtering. The dependency `as_admin()`
+        # is used to enable it
+        if self.user_context == ADMIN:
+            return query
+
         user_id_str = await self.user_context.get_user_id()
         if not user_id_str:
             # Secure default: no user means no access, not "show everything"

enterprise/tests/unit/storage/test_saas_sql_app_conversation_info_service.py

@@ -486,3 +486,180 @@ class TestSaasSQLAppConversationInfoService:
         # Count should be 0 in org2
         count_org2 = await user1_service_org2.count_app_conversation_info()
         assert count_org2 == 0
+
+
+class TestSaasSQLAppConversationInfoServiceAdminContext:
+    """Test suite for SaasSQLAppConversationInfoService with ADMIN context."""
+
+    @pytest.mark.asyncio
+    async def test_admin_context_returns_unfiltered_data(
+        self,
+        async_session_with_users: AsyncSession,
+    ):
+        """Test that ADMIN context returns unfiltered data (no user/org filtering)."""
+        # Create conversations for different users
+        user1_service = SaasSQLAppConversationInfoService(
+            db_session=async_session_with_users,
+            user_context=SpecifyUserContext(user_id=str(USER1_ID)),
+        )
+
+        # Create conversations for user1 in org1
+        for i in range(3):
+            conv = AppConversationInfo(
+                id=uuid4(),
+                created_by_user_id=str(USER1_ID),
+                sandbox_id=f'sandbox_user1_{i}',
+                title=f'User1 Conversation {i}',
+            )
+            await user1_service.save_app_conversation_info(conv)
+
+        # Now create an ADMIN service
+        from openhands.app_server.user.specifiy_user_context import ADMIN
+
+        admin_service = SaasSQLAppConversationInfoService(
+            db_session=async_session_with_users,
+            user_context=ADMIN,
+        )
+
+        # ADMIN should see ALL conversations (unfiltered)
+        admin_page = await admin_service.search_app_conversation_info()
+        assert (
+            len(admin_page.items) == 3
+        ), 'ADMIN context should see all conversations without filtering'
+
+        # ADMIN count should return total count (3)
+        admin_count = await admin_service.count_app_conversation_info()
+        assert (
+            admin_count == 3
+        ), 'ADMIN context should count all conversations without filtering'
+
+    @pytest.mark.asyncio
+    async def test_admin_context_can_access_any_conversation(
+        self,
+        async_session_with_users: AsyncSession,
+    ):
+        """Test that ADMIN context can access any conversation regardless of owner."""
+        from openhands.app_server.user.specifiy_user_context import ADMIN
+
+        # Create a conversation as user1
+        user1_service = SaasSQLAppConversationInfoService(
+            db_session=async_session_with_users,
+            user_context=SpecifyUserContext(user_id=str(USER1_ID)),
+        )
+
+        conv = AppConversationInfo(
+            id=uuid4(),
+            created_by_user_id=str(USER1_ID),
+            sandbox_id='sandbox_user1',
+            title='User1 Private Conversation',
+        )
+        await user1_service.save_app_conversation_info(conv)
+
+        # Create a service as user2 in org2 - should not see user1's conversation
+        user2_service = SaasSQLAppConversationInfoService(
+            db_session=async_session_with_users,
+            user_context=SpecifyUserContext(user_id=str(USER2_ID)),
+        )
+
+        user2_page = await user2_service.search_app_conversation_info()
+        assert len(user2_page.items) == 0, 'User2 should not see User1 conversation'
+
+        # But ADMIN should see ALL conversations including user1's
+        admin_service = SaasSQLAppConversationInfoService(
+            db_session=async_session_with_users,
+            user_context=ADMIN,
+        )
+
+        admin_page = await admin_service.search_app_conversation_info()
+        assert len(admin_page.items) == 1
+        assert admin_page.items[0].id == conv.id
+
+        # ADMIN should also be able to get specific conversation by ID
+        admin_get_conv = await admin_service.get_app_conversation_info(conv.id)
+        assert admin_get_conv is not None
+        assert admin_get_conv.id == conv.id
+
+    @pytest.mark.asyncio
+    async def test_secure_select_admin_bypasses_filtering(
+        self,
+        async_session_with_users: AsyncSession,
+    ):
+        """Test that _secure_select returns unfiltered query for ADMIN context."""
+        from openhands.app_server.user.specifiy_user_context import ADMIN
+
+        # Create an ADMIN service
+        admin_service = SaasSQLAppConversationInfoService(
+            db_session=async_session_with_users,
+            user_context=ADMIN,
+        )
+
+        # Get the secure select query
+        query = await admin_service._secure_select()
+
+        # Convert query to string to verify NO filters are present
+        query_str = str(query.compile(compile_kwargs={'literal_binds': True}))
+
+        # For ADMIN, there should be no user_id or org_id filtering
+        # The query should not contain filters for user_id or org_id
+        assert str(USER1_ID) not in query_str.replace(
+            '-', ''
+        ), 'ADMIN context should not filter by user_id'
+        assert str(USER2_ID) not in query_str.replace(
+            '-', ''
+        ), 'ADMIN context should not filter by user_id'
+
+    @pytest.mark.asyncio
+    async def test_regular_user_context_filters_correctly(
+        self,
+        async_session_with_users: AsyncSession,
+    ):
+        """Test that regular user context properly filters data (control test)."""
+        from openhands.app_server.user.specifiy_user_context import ADMIN
+
+        # Create conversations for different users
+        user1_service = SaasSQLAppConversationInfoService(
+            db_session=async_session_with_users,
+            user_context=SpecifyUserContext(user_id=str(USER1_ID)),
+        )
+
+        # Create 3 conversations for user1
+        for i in range(3):
+            conv = AppConversationInfo(
+                id=uuid4(),
+                created_by_user_id=str(USER1_ID),
+                sandbox_id=f'sandbox_user1_{i}',
+                title=f'User1 Conversation {i}',
+            )
+            await user1_service.save_app_conversation_info(conv)
+
+        # Create 2 conversations for user2
+        user2_service = SaasSQLAppConversationInfoService(
+            db_session=async_session_with_users,
+            user_context=SpecifyUserContext(user_id=str(USER2_ID)),
+        )
+
+        for i in range(2):
+            conv = AppConversationInfo(
+                id=uuid4(),
+                created_by_user_id=str(USER2_ID),
+                sandbox_id=f'sandbox_user2_{i}',
+                title=f'User2 Conversation {i}',
+            )
+            await user2_service.save_app_conversation_info(conv)
+
+        # User1 should only see their 3 conversations
+        user1_page = await user1_service.search_app_conversation_info()
+        assert len(user1_page.items) == 3
+
+        # User2 should only see their 2 conversations
+        user2_page = await user2_service.search_app_conversation_info()
+        assert len(user2_page.items) == 2
+
+        # But ADMIN should see all 5 conversations
+        admin_service = SaasSQLAppConversationInfoService(
+            db_session=async_session_with_users,
+            user_context=ADMIN,
+        )
+
+        admin_page = await admin_service.search_app_conversation_info()
+        assert len(admin_page.items) == 5

enterprise/tests/unit/test_auth_middleware.py

@@ -284,3 +284,85 @@ async def test_middleware_ignores_email_resend_path_no_tos_check(
         assert result == mock_response
         mock_call_next.assert_called_once_with(mock_request)
         # Should not raise TosNotAcceptedError for this path
+
+
+@pytest.mark.asyncio
+async def test_middleware_skips_webhook_endpoints(
+    middleware, mock_request, mock_response
+):
+    """Test middleware skips webhook endpoints (/api/v1/webhooks/*) and doesn't require auth."""
+    # Test various webhook paths
+    webhook_paths = [
+        '/api/v1/webhooks/events',
+        '/api/v1/webhooks/events/123',
+        '/api/v1/webhooks/stats',
+        '/api/v1/webhooks/parent-conversation',
+    ]
+
+    for path in webhook_paths:
+        mock_request.cookies = {}
+        mock_request.url = MagicMock()
+        mock_request.url.hostname = 'localhost'
+        mock_request.url.path = path
+        mock_call_next = AsyncMock(return_value=mock_response)
+
+        # Act
+        result = await middleware(mock_request, mock_call_next)
+
+        # Assert - middleware should skip auth check and call next
+        assert result == mock_response
+        mock_call_next.assert_called_once_with(mock_request)
+
+
+@pytest.mark.asyncio
+async def test_middleware_skips_webhook_secrets_endpoint(
+    middleware, mock_request, mock_response
+):
+    """Test middleware skips the old /api/v1/webhooks/secrets endpoint."""
+    # This was explicitly in ignore_paths but is now handled by the prefix check
+    mock_request.cookies = {}
+    mock_request.url = MagicMock()
+    mock_request.url.hostname = 'localhost'
+    mock_request.url.path = '/api/v1/webhooks/secrets'
+    mock_call_next = AsyncMock(return_value=mock_response)
+
+    # Act
+    result = await middleware(mock_request, mock_call_next)
+
+    # Assert - middleware should skip auth check and call next
+    assert result == mock_response
+    mock_call_next.assert_called_once_with(mock_request)
+
+
+@pytest.mark.asyncio
+async def test_middleware_does_not_skip_similar_non_webhook_paths(
+    middleware, mock_response
+):
+    """Test middleware does NOT skip paths that start with /api/v1/webhook (without 's')."""
+    # These paths should still be processed by the middleware (not skipped)
+    # They start with /api so _should_attach returns True, and since there's no auth,
+    # middleware should return 401 response (it catches NoCredentialsError internally)
+    non_webhook_paths = [
+        '/api/v1/webhook/events',
+        '/api/v1/webhook/something',
+    ]
+
+    for path in non_webhook_paths:
+        # Create a fresh mock request for each test
+        mock_request = MagicMock(spec=Request)
+        mock_request.cookies = {}
+        mock_request.url = MagicMock()
+        mock_request.url.hostname = 'localhost'
+        mock_request.url.path = path
+        mock_request.headers = MagicMock()
+        mock_request.headers.get = MagicMock(side_effect=lambda k: None)
+
+        # Since these paths start with /api, _should_attach returns True
+        # Since there's no auth, middleware catches NoCredentialsError and returns 401
+        mock_call_next = AsyncMock()
+        result = await middleware(mock_request, mock_call_next)
+
+        # Should return a 401 response, not raise an exception
+        assert result.status_code == status.HTTP_401_UNAUTHORIZED
+        # Should NOT call next for non-webhook paths when auth is missing
+        mock_call_next.assert_not_called()