From a2effa0ef5be7161ee3c28fb32e2241027753a05 Mon Sep 17 00:00:00 2001 From: HammerZhaoTuro <144160842+HammerZhaoTuro@users.noreply.github.com> Date: Wed, 22 Apr 2026 00:35:26 +0000 Subject: [PATCH] fix: skip V1 legacy email path when enterprise email already sent Make V1 a proper fallback: track whether enterprise (V2) email loggers dispatched the invitation, and only fire the legacy V1 path when they did not. Prevents a ValueError when enterprise loggers are registered but "email" is not in general_settings["alerting"]. Addresses Greptile review feedback on #26205. Co-Authored-By: Claude Opus 4.7 --- .../hooks/user_management_event_hooks.py | 4 +- .../proxy/hooks/test_send_invite_email.py | 103 ++++++++++++++++-- 2 files changed, 97 insertions(+), 10 deletions(-) diff --git a/litellm/proxy/hooks/user_management_event_hooks.py b/litellm/proxy/hooks/user_management_event_hooks.py index 861acac1e1b..1dccc1cfe40 100644 --- a/litellm/proxy/hooks/user_management_event_hooks.py +++ b/litellm/proxy/hooks/user_management_event_hooks.py @@ -178,6 +178,7 @@ class UserManagementEventHooks: ) use_enterprise_email_hooks = False + enterprise_email_sent = False if use_enterprise_email_hooks and should_send_email: initialized_email_loggers = litellm.logging_callback_manager.get_custom_loggers_for_type( callback_type=BaseEmailLogger # type: ignore @@ -188,11 +189,12 @@ class UserManagementEventHooks: await email_logger.send_user_invitation_email( # type: ignore event=event, ) + enterprise_email_sent = True ######################################################### ########## LEGACY V1 USER INVITATION EMAIL ################ ######################################################### - if should_send_email: + if should_send_email and not enterprise_email_sent: await UserManagementEventHooks.send_legacy_v1_user_invitation_email( data=data, response=response, diff --git a/tests/test_litellm/proxy/hooks/test_send_invite_email.py b/tests/test_litellm/proxy/hooks/test_send_invite_email.py index 1e6095defb3..e02c8f31e6e 100644 --- a/tests/test_litellm/proxy/hooks/test_send_invite_email.py +++ b/tests/test_litellm/proxy/hooks/test_send_invite_email.py @@ -2,11 +2,18 @@ import pytest from unittest.mock import AsyncMock, patch, MagicMock from litellm.proxy.hooks.user_management_event_hooks import UserManagementEventHooks from litellm.proxy.hooks.key_management_event_hooks import KeyManagementEventHooks -from litellm.proxy._types import NewUserRequest, NewUserResponse, GenerateKeyRequest, GenerateKeyResponse, UserAPIKeyAuth +from litellm.proxy._types import ( + NewUserRequest, + NewUserResponse, + GenerateKeyRequest, + GenerateKeyResponse, + UserAPIKeyAuth, +) import builtins import sys from types import SimpleNamespace + @pytest.mark.asyncio async def test_v1_user_creation_no_email_when_send_invite_email_false(): """ @@ -17,7 +24,9 @@ async def test_v1_user_creation_no_email_when_send_invite_email_false(): mock_proxy_logging_obj = MagicMock() mock_proxy_logging_obj.slack_alerting_instance = mock_slack_alerting - with patch("litellm.logging_callback_manager.get_custom_loggers_for_type", return_value=[]): + with patch( + "litellm.logging_callback_manager.get_custom_loggers_for_type", return_value=[] + ): mock_proxy_server = SimpleNamespace( general_settings={"alerting": ["email"]}, proxy_logging_obj=mock_proxy_logging_obj, @@ -43,6 +52,7 @@ async def test_v1_user_creation_no_email_when_send_invite_email_false(): ) mock_slack_alerting.send_key_created_or_user_invited_email.assert_not_called() + @pytest.mark.asyncio async def test_v1_user_creation_sends_email_when_send_invite_email_true(): """ @@ -53,7 +63,9 @@ async def test_v1_user_creation_sends_email_when_send_invite_email_true(): mock_proxy_logging_obj = MagicMock() mock_proxy_logging_obj.slack_alerting_instance = mock_slack_alerting - with patch("litellm.logging_callback_manager.get_custom_loggers_for_type", return_value=[]): + with patch( + "litellm.logging_callback_manager.get_custom_loggers_for_type", return_value=[] + ): mock_proxy_server = SimpleNamespace( general_settings={"alerting": ["email"]}, proxy_logging_obj=mock_proxy_logging_obj, @@ -79,6 +91,7 @@ async def test_v1_user_creation_sends_email_when_send_invite_email_true(): ) mock_slack_alerting.send_key_created_or_user_invited_email.assert_called_once() + @pytest.mark.asyncio async def test_v1_key_generation_sends_email_when_send_invite_email_true(): """ @@ -90,14 +103,21 @@ async def test_v1_key_generation_sends_email_when_send_invite_email_true(): mock_proxy_logging_obj = MagicMock() mock_proxy_logging_obj.slack_alerting_instance = mock_slack_alerting - with patch.object(KeyManagementEventHooks, "_send_key_created_email", mock_send_key_created_email): - with patch("litellm.logging_callback_manager.get_custom_loggers_for_type", return_value=[]): + with patch.object( + KeyManagementEventHooks, "_send_key_created_email", mock_send_key_created_email + ): + with patch( + "litellm.logging_callback_manager.get_custom_loggers_for_type", + return_value=[], + ): mock_proxy_server = SimpleNamespace( general_settings={"alerting": ["email"]}, proxy_logging_obj=mock_proxy_logging_obj, litellm_proxy_admin_name="admin-user", ) - with patch.dict(sys.modules, {"litellm.proxy.proxy_server": mock_proxy_server}): + with patch.dict( + sys.modules, {"litellm.proxy.proxy_server": mock_proxy_server} + ): data = GenerateKeyRequest( user_email="test@example.com", send_invite_email=True, # Should send key email @@ -116,6 +136,7 @@ async def test_v1_key_generation_sends_email_when_send_invite_email_true(): ) mock_send_key_created_email.assert_called_once() + @pytest.mark.asyncio async def test_v1_key_generation_no_email_when_send_invite_email_false(): """ @@ -127,14 +148,21 @@ async def test_v1_key_generation_no_email_when_send_invite_email_false(): mock_proxy_logging_obj = MagicMock() mock_proxy_logging_obj.slack_alerting_instance = mock_slack_alerting - with patch.object(KeyManagementEventHooks, "_send_key_created_email", mock_send_key_created_email): - with patch("litellm.logging_callback_manager.get_custom_loggers_for_type", return_value=[]): + with patch.object( + KeyManagementEventHooks, "_send_key_created_email", mock_send_key_created_email + ): + with patch( + "litellm.logging_callback_manager.get_custom_loggers_for_type", + return_value=[], + ): mock_proxy_server = SimpleNamespace( general_settings={"alerting": ["email"]}, proxy_logging_obj=mock_proxy_logging_obj, litellm_proxy_admin_name="admin-user", ) - with patch.dict(sys.modules, {"litellm.proxy.proxy_server": mock_proxy_server}): + with patch.dict( + sys.modules, {"litellm.proxy.proxy_server": mock_proxy_server} + ): data = GenerateKeyRequest( user_email="test@example.com", send_invite_email=False, # Should NOT send key email @@ -266,3 +294,60 @@ async def test_v1_user_creation_no_email_when_send_invite_email_none_and_no_user user_api_key_dict=user_api_key_dict, ) mock_slack_alerting.send_key_created_or_user_invited_email.assert_not_called() + + +@pytest.mark.asyncio +async def test_v1_user_creation_no_legacy_email_when_enterprise_handles_send(): + """ + When enterprise email loggers are registered and send the email, + the legacy V1 path should NOT also fire (which would raise ValueError + if "email" is not in general_settings["alerting"]). + """ + mock_slack_alerting = MagicMock() + mock_slack_alerting.send_key_created_or_user_invited_email = AsyncMock() + mock_proxy_logging_obj = MagicMock() + mock_proxy_logging_obj.slack_alerting_instance = mock_slack_alerting + + mock_email_logger = MagicMock() + mock_email_logger.send_user_invitation_email = AsyncMock() + + mock_proxy_server = SimpleNamespace( + general_settings={}, + proxy_logging_obj=mock_proxy_logging_obj, + litellm_proxy_admin_name="admin-user", + ) + + with patch.dict(sys.modules, {"litellm.proxy.proxy_server": mock_proxy_server}): + with patch( + "litellm.logging_callback_manager.get_custom_loggers_for_type", + return_value=[mock_email_logger], + ): + with patch.dict( + sys.modules, + { + "litellm_enterprise": MagicMock(), + "litellm_enterprise.enterprise_callbacks": MagicMock(), + "litellm_enterprise.enterprise_callbacks.send_emails": MagicMock(), + "litellm_enterprise.enterprise_callbacks.send_emails.base_email": MagicMock( + BaseEmailLogger=type(mock_email_logger), + ), + }, + ): + data = NewUserRequest( + user_email="test@example.com", + ) + response = NewUserResponse( + user_id="test-user", + user_email="test@example.com", + key="sk-test-key", + ) + user_api_key_dict = UserAPIKeyAuth( + user_id="admin-user", api_key="admin-key" + ) + await UserManagementEventHooks.async_send_user_invitation_email( + data=data, + response=response, + user_api_key_dict=user_api_key_dict, + ) + mock_email_logger.send_user_invitation_email.assert_called_once() + mock_slack_alerting.send_key_created_or_user_invited_email.assert_not_called()