diff --git a/app/interface/auth/InterfaceAuthUser.py b/app/interface/auth/InterfaceAuthUser.py index a1ae2517..4a00afd9 100644 --- a/app/interface/auth/InterfaceAuthUser.py +++ b/app/interface/auth/InterfaceAuthUser.py @@ -3,17 +3,20 @@ from app.auth.User import UserAnonymous from app.config.settings.SystemSettings import SystemSettingsObj, SystemSettings -from app.config.settings.DomainSettings import AuthSettingsObj, AuthSettings, UserSourceSettings, UserSourceSettingsObj +from app.config.settings.DomainSettings import AuthSettingsObj, AuthSettings, UserSourceSettings, UserSourceSettingsObj, MailSettings, MailSettingsObj from app.config.settings.UserSettings import UserGeneralSettings from app.module.auth.ModuleAuth import ModuleAuth from app.module.auth.ModuleUserSource import ModuleUserSource from app.module.calendar.ModuleCalendar import ModuleCalendar from app.module.contact.ModuleContact import ModuleContact +from app.module.mail.ModuleMail import ModuleMail from app.module.user.ModuleUserProfile import ModuleUserProfile from app.utils.api.ApiBaseResponse import create_api_base_response from app.utils.exceptions import RequestException, BugException from app.utils import errors as err from app.utils.logger.logger import logger_api +from app.utils import constants as cs + if TYPE_CHECKING: from app.config.settings.ProcessSetting import ProcessSetting @@ -34,12 +37,13 @@ def __init__(self, process: ProcessSetting, system: dict, default_domain: dict): for source_uid, source_settings in default_us_source_raw.items(): default_us_source[source_uid] = UserSourceSettingsObj(source_settings) + self.process = process + self.default_domain = default_domain self.module_auth = ModuleAuth(process, system_settings, default_auth, default_us_source) self.module_user_profile = ModuleUserProfile(process, default_domain) self._module_calendar: ModuleCalendar = ModuleCalendar(process) self._module_contact: ModuleContact = ModuleContact(process) - def get_login_mech(self, user_uid:str, redirect:str) -> tuple[dict, int]: """ Get the login mech from a uid @@ -103,6 +107,8 @@ def plain_login(self, data:dict) -> tuple[dict, int]: user_tz: str = raw_gen.get(UserGeneralSettings.subparent, {}).get("SOGO_U_TIMEZONE", "UTC") self._module_calendar.create_personal_calendar(uid, tz=user_tz) self._module_contact.create_personal_addressbook(uid) + module_mail = ModuleMail(user, MailSettingsObj(self.default_domain[MailSettings.subparent]), self.process) + module_mail.create_special_folders_if_not_exist(cs.DEFAULT_IDENTITY_KEY_VALUE) except RequestException as ex: logger_api.error("Request exception when onboarding user %s: %s", uid, str(ex)) return create_api_base_response(None, ex.error) diff --git a/app/manager/mail/ClientImap.py b/app/manager/mail/ClientImap.py index 327d48a0..765cff70 100644 --- a/app/manager/mail/ClientImap.py +++ b/app/manager/mail/ClientImap.py @@ -1089,14 +1089,28 @@ def uid_copy(self, mail_uid: str|list|Iterator, dest_mailbox: str) -> None: raise RequestException(f"Mailbox name is not ascii: {dest_mailbox}", err.ERROR_IMAP_NOT_ASCII) if isinstance(mail_uid, (Iterator, list)): mail_uid = ','.join(mail_uid) - dest_mailbox = quote(dest_mailbox) - success, datas = self._exec_imap4_method(self.connection.uid, 'COPY', mail_uid, dest_mailbox) + dest_mailbox_quoted = quote(dest_mailbox) + success, datas = self._exec_imap4_method(self.connection.uid, 'COPY', mail_uid, dest_mailbox_quoted) # Beware, if the uid does not exist, IMAP4 still return OK with data to None. Not a big problem, though. if not success: if datas[0].decode().startswith("[TRYCREATE]"): - raise RequestException(f"Folder '{dest_mailbox}' does not exist", err.ERROR_FOLDER_NAME_NOT_FOUND) - logger_imap.error("UID COPY failed for UID %s to %s", mail_uid, dest_mailbox) - raise RequestException(f"UID COPY failed for UID {mail_uid} to {dest_mailbox}", err.ERROR_IMAP_FAILED) + # Get folder type from the unquoted path + folder_type = self.folders_map_name_to_type.get(dest_mailbox, cs.MAIL_FOLDER_NORMAL) + + # Create folder if it's not of type NORMAL + if folder_type != cs.MAIL_FOLDER_NORMAL: + logger_imap.info(f"Folder '{dest_mailbox}' does not exist but is of special type '{folder_type}', creating it") + self._imap_create_folder(dest_mailbox_quoted, auto_sub=True, no_error_if_exist=True) + # Retry the copy after folder creation + success, datas = self._exec_imap4_method(self.connection.uid, 'COPY', mail_uid, dest_mailbox_quoted) + if not success: + logger_imap.error(f"UID COPY failed for UID {mail_uid} to {dest_mailbox} after folder creation") + raise RequestException(f"UID COPY failed for UID {mail_uid} to {dest_mailbox}", err.ERROR_IMAP_FAILED) + else: + raise RequestException(f"Folder '{dest_mailbox}' does not exist", err.ERROR_FOLDER_NAME_NOT_FOUND) + else: + logger_imap.error(f"UID COPY failed for UID {mail_uid} to {dest_mailbox}") + raise RequestException(f"UID COPY failed for UID {mail_uid} to {dest_mailbox}", err.ERROR_IMAP_FAILED) else: raise BugException("Not authenticated meaning self.connect() and self.login() was not called beforehands") @@ -1918,10 +1932,28 @@ def save_mail_to_folder(self, message: EmailMessage, folder_type: str, flags: st raw_bytes, ) if not success: - raise RequestException( - f"Failed to append mail to folder '{folder_path}': {datas}", - err.ERROR_MAIL_SAVE_SENT_FAILED, - ) + # Create folder if it's not of type NORMAL and retry + if folder_type != cs.MAIL_FOLDER_NORMAL: + logger_imap.info(f"Folder '{folder_path}' (type '{folder_type}') does not exist, creating it") + self._imap_create_folder(quoted_folder, auto_sub=True, no_error_if_exist=True) + # Retry the append after folder creation + success, datas = self._exec_imap4_method( + self.connection.append, # type: ignore[arg-type] + quoted_folder, + flags, + None, # type: ignore[arg-type] + raw_bytes, + ) + if not success: + raise RequestException( + f"Failed to append mail to folder '{folder_path}' after creation: {datas}", + err.ERROR_MAIL_SAVE_SENT_FAILED, + ) + else: + raise RequestException( + f"Failed to append mail to folder '{folder_path}': {datas}", + err.ERROR_MAIL_SAVE_SENT_FAILED, + ) logger_imap.info("Mail saved in folder '%s'", folder_path) def delete_mail_permanently_from_folder_type(self, folder_type: str, mail_uid: str) -> None: diff --git a/app/module/mail/ModuleMail.py b/app/module/mail/ModuleMail.py index bd989fad..ee62b337 100644 --- a/app/module/mail/ModuleMail.py +++ b/app/module/mail/ModuleMail.py @@ -165,6 +165,46 @@ def create_folder(self, account_id:str, folder_name: str, parent_path: str) -> d new_folder_path = client.create_folder(folder_name, parent_path) return client.get_one_folder(new_folder_path) + def create_special_folders_if_not_exist(self, account_id: str) -> None: + """Create special mail folders (Sent, Draft, Junk, Trash, Template) if they don't exist. + + This is typically called during user first login to ensure all special folders are available. + INBOX and NORMAL folders are excluded from creation as they are typically handled differently. + + :param account_id: The account identifier ("0" for main, hash for external) + :type account_id: str + :raises RequestException: If connection or manager operations fail + """ + client = self._open_client_for(account_id) + + # Special folder types to create (excluding NORMAL and INBOX) + special_folder_types = [ + cs.MAIL_FOLDER_SENT, + cs.MAIL_FOLDER_DRAFT, + cs.MAIL_FOLDER_JUNK, + cs.MAIL_FOLDER_TRASH, + cs.MAIL_FOLDER_TEMPLATE, + ] + + # Get list of existing folders + existing_folders = client.list_folders() + existing_types = {folder.get("type") for folder in existing_folders} + + # Create missing special folders + for folder_type in special_folder_types: + if folder_type not in existing_types: + try: + client.create_folder(folder_type, "") + logger_mail_server.info( + "Created special folder '%s' for account '%s'", + folder_type, account_id + ) + except RequestException as e: + logger_mail_server.warning( + "Failed to create special folder '%s' for account '%s': %s", + folder_type, account_id, str(e) + ) + def delete_folder(self, account_id: str, folder_path: str, do_children:bool = True) -> None: """Delete a mail folder. diff --git a/tests/test_interface/test_auth/test_InterfaceAuthUser.py b/tests/test_interface/test_auth/test_InterfaceAuthUser.py index d141d333..a83b4a41 100644 --- a/tests/test_interface/test_auth/test_InterfaceAuthUser.py +++ b/tests/test_interface/test_auth/test_InterfaceAuthUser.py @@ -124,6 +124,28 @@ def create_personal_addressbook(self, user_uid, name="Personal contacts"): self.create_personal_addressbook_args = user_uid +class FakeUser: + """Fake User object for testing.""" + def __init__(self, uid, password): + self.uid = uid + self.password = password + self.mail = uid + self.source_id = "source1" + + +class FakeModuleMail: + """Fake ModuleMail for testing.""" + def __init__(self, user, mail_settings, process): + self.user = user + self.mail_settings = mail_settings + self.process = process + self.create_special_folders_args = None + + def create_special_folders_if_not_exist(self, account_id): + """Create special folders if they don't exist.""" + self.create_special_folders_args = account_id + + def patch_modules_on_interface(monkeypatch, fake_module_auth, fake_module_user_profile, fake_module_user_source_class): """Patch modules in InterfaceAuthUser.""" monkeypatch.setattr( @@ -146,6 +168,10 @@ def patch_modules_on_interface(monkeypatch, fake_module_auth, fake_module_user_p "app.interface.auth.InterfaceAuthUser.ModuleContact", FakeModuleContact ) + monkeypatch.setattr( + "app.interface.auth.InterfaceAuthUser.ModuleMail", + FakeModuleMail + ) # ========== Tests for get_login_mech ========== @@ -197,7 +223,7 @@ def test_get_login_mech_request_exception(monkeypatch): def test_plain_login_success(monkeypatch): """Test successful plain login.""" fake_auth = FakeModuleAuth(None, None, None, None) - fake_user = {"uid": "testuser@example.com", "password": "secret123"} + fake_user = FakeUser("testuser@example.com", "secret123") fake_auth.get_user_and_domain_user_sources_result = (fake_user, {"source1": {}}) fake_profile = FakeModuleUserProfile(None, None) @@ -227,7 +253,7 @@ def fake_us_class(sources): def test_plain_login_failed_authentication(monkeypatch): """Test failed authentication.""" fake_auth = FakeModuleAuth(None, None, None, None) - fake_user = {"uid": "testuser@example.com", "password": "wrong"} + fake_user = FakeUser("testuser@example.com", "wrong") fake_auth.get_user_and_domain_user_sources_result = (fake_user, {"source1": {}}) fake_profile = FakeModuleUserProfile(None, None) @@ -254,7 +280,7 @@ def fake_us_class(sources): def test_plain_login_create_user_profile(monkeypatch): """Test plain login creates user profile and personal calendar on first login.""" fake_auth = FakeModuleAuth(None, None, None, None) - fake_user = {"uid": "newuser@example.com", "password": "secret123"} + fake_user = FakeUser("newuser@example.com", "secret123") fake_auth.get_user_and_domain_user_sources_result = (fake_user, {"source1": {}}) fake_profile = FakeModuleUserProfile(None, None) @@ -269,7 +295,7 @@ def fake_us_class(sources): interface = InterfaceAuthUser( process={"test": "config"}, system={"SYSTEM_SETTINGS": {"test": "value"}}, - default_domain={"AUTH_SETTINGS": {"test": "value"}, "USER_SOURCE": {}} + default_domain={"AUTH_SETTINGS": {"test": "value"}, "USER_SOURCE": {}, "MAIL_SETTINGS": {"test": "value"}} ) data = {"username": "newuser@example.com", "password": "secret123"} @@ -277,7 +303,7 @@ def fake_us_class(sources): assert status_code == 200 assert fake_profile.create_user_profile_args is not None - assert fake_profile.create_user_profile_args["uid"] == "newuser@example.com" + assert fake_profile.create_user_profile_args.uid == "newuser@example.com" assert interface._module_calendar.create_personal_calendar_args == "newuser@example.com" # pylint: disable=protected-access # The user's preferred timezone is forwarded to the default calendar. assert interface._module_calendar.create_personal_calendar_timezone == "Europe/Paris" # pylint: disable=protected-access @@ -288,7 +314,7 @@ def fake_us_class(sources): def test_plain_login_profile_creation_request_exception(monkeypatch): """Test error handling when user profile creation fails with RequestException.""" fake_auth = FakeModuleAuth(None, None, None, None) - fake_user = {"uid": "newuser@example.com", "password": "secret123"} + fake_user = FakeUser("newuser@example.com", "secret123") fake_auth.get_user_and_domain_user_sources_result = (fake_user, {"source1": {}}) fake_profile = FakeModuleUserProfile(None, None) @@ -306,7 +332,7 @@ def fake_us_class(sources): interface = InterfaceAuthUser( process={"test": "config"}, system={"SYSTEM_SETTINGS": {"test": "value"}}, - default_domain={"AUTH_SETTINGS": {"test": "value"}, "USER_SOURCE": {}} + default_domain={"AUTH_SETTINGS": {"test": "value"}, "USER_SOURCE": {}, "MAIL_SETTINGS": {"test": "value"}} ) data = {"username": "newuser@example.com", "password": "secret123"} @@ -320,7 +346,7 @@ def fake_us_class(sources): def test_plain_login_profile_creation_bug_exception(monkeypatch): """Test error handling when user profile creation fails with BugException.""" fake_auth = FakeModuleAuth(None, None, None, None) - fake_user = {"uid": "newuser@example.com", "password": "secret123"} + fake_user = FakeUser("newuser@example.com", "secret123") fake_auth.get_user_and_domain_user_sources_result = (fake_user, {"source1": {}}) fake_profile = FakeModuleUserProfile(None, None) @@ -338,7 +364,7 @@ def fake_us_class(sources): interface = InterfaceAuthUser( process={"test": "config"}, system={"SYSTEM_SETTINGS": {"test": "value"}}, - default_domain={"AUTH_SETTINGS": {"test": "value"}, "USER_SOURCE": {}} + default_domain={"AUTH_SETTINGS": {"test": "value"}, "USER_SOURCE": {}, "MAIL_SETTINGS": {"test": "value"}} ) data = {"username": "newuser@example.com", "password": "secret123"} diff --git a/tests/test_manager/test_mail/test_clientImap.py b/tests/test_manager/test_mail/test_clientImap.py index 45163d3e..4bc00b64 100644 --- a/tests/test_manager/test_mail/test_clientImap.py +++ b/tests/test_manager/test_mail/test_clientImap.py @@ -80,6 +80,7 @@ def __init__(self): b'(\\HasNoChildren) "." "Sent"']) self.expunge_response = ("OK", [b"1", b"2"]) self.uid_response = ("OK", [b""]) + self.append_response = ("OK", [b""]) self.getacl_response = ("OK", [b"INBOX user1 lrswipkxtea user2 lr"]) self.setacl_response = ("OK", [b""]) self.deleteacl_response = ("OK", [b""]) @@ -158,6 +159,9 @@ def xatom(self, *args): return ("OK", [b""]) # --- mails --- + def append(self, mailbox, flags, date_time, message): + return self.append_response + def expunge(self): return self.expunge_response @@ -843,6 +847,41 @@ def test_uid_copy_dest_not_exist_raises(self): with pytest.raises(RequestException): client.uid_copy("100", "NoSuchFolder") + def test_uid_copy_special_folder_creates_and_retries(self): + """Test that uid_copy creates a special folder (SENT) if it doesn't exist and retries.""" + fake_conn = FakeIMAPConnection() + # First call to uid_copy fails with TRYCREATE, second succeeds + fake_conn.uid_response = ("NO", [b"[TRYCREATE] No such mailbox"]) + client = authenticated_client(fake_conn) + + # Mock _imap_create_folder to track calls and change uid_response for the retry + with mock.patch.object(client, "_imap_create_folder") as mock_create: + # Setup: first uid call fails, second succeeds + uid_responses = [ + ("NO", [b"[TRYCREATE] No such mailbox"]), + ("OK", [b""]) + ] + call_count = 0 + def uid_response_side_effect(*args, **kwargs): + nonlocal call_count + result = uid_responses[call_count % len(uid_responses)] + call_count += 1 + return result + + with mock.patch.object(fake_conn, "uid", side_effect=uid_response_side_effect): + # "Sent" is a MAIL_FOLDER_SENT which is special, not NORMAL + client.uid_copy("100", "Sent") + mock_create.assert_called_once() + + def test_uid_copy_normal_folder_does_not_create(self): + """Test that uid_copy does NOT create a NORMAL folder if it doesn't exist.""" + fake_conn = FakeIMAPConnection() + fake_conn.uid_response = ("NO", [b"[TRYCREATE] No such mailbox"]) + client = authenticated_client(fake_conn) + # "NoSuchFolder" is not in folders_map, so it will be NORMAL type + with pytest.raises(RequestException): + client.uid_copy("100", "NoSuchFolder") + def test_uid_copy_not_authenticated_raises(self): client = make_client() client.connection = None @@ -1122,3 +1161,115 @@ def test_copy_not_authenticated_raises(self): client.connection = None with pytest.raises(BugException): client.copy_mail_to_mailbox("INBOX", "100", "Sent") + + +# =========================================================================== +# Tests: save_mail_to_folder +# =========================================================================== + +class TestSaveMailToFolder: + def test_save_mail_to_sent_folder_success(self): + """Test saving a mail to SENT folder successfully.""" + from email.message import EmailMessage + fake_conn = FakeIMAPConnection() + fake_conn.uid_response = ("OK", [b""]) + client = authenticated_client(fake_conn) + + msg = EmailMessage() + msg["Subject"] = "Test" + msg.set_content("Test body") + + client.save_mail_to_folder(msg, cs.MAIL_FOLDER_SENT) + # No exception means success + + def test_save_mail_special_folder_creates_and_retries(self): + """Test that save_mail_to_folder creates a special folder (SENT) if it doesn't exist and retries.""" + from email.message import EmailMessage + fake_conn = FakeIMAPConnection() + client = authenticated_client(fake_conn) + + msg = EmailMessage() + msg["Subject"] = "Test" + msg.set_content("Test body") + + # Setup: first append call fails, second succeeds + append_responses = [ + ("NO", [b"[TRYCREATE] No such mailbox"]), + ("OK", [b""]) + ] + call_count = 0 + def append_response_side_effect(*args, **kwargs): + nonlocal call_count + result = append_responses[call_count % len(append_responses)] + call_count += 1 + return result + + with mock.patch.object(client, "_imap_create_folder") as mock_create: + with mock.patch.object(fake_conn, "append", side_effect=append_response_side_effect): + # MAIL_FOLDER_SENT is a special folder, not NORMAL + client.save_mail_to_folder(msg, cs.MAIL_FOLDER_SENT) + mock_create.assert_called_once() + + def test_save_mail_normal_folder_does_not_create(self): + """Test that save_mail_to_folder does NOT create a NORMAL folder if it doesn't exist.""" + from email.message import EmailMessage + fake_conn = FakeIMAPConnection() + # Setup folders_map with a NORMAL folder + folders_map = { + cs.MAIL_FOLDER_INBOX: "INBOX", + cs.MAIL_FOLDER_SENT: "Sent", + cs.MAIL_FOLDER_NORMAL: "CustomFolder", # This is a NORMAL folder type + } + client = make_client(folders_map=folders_map) + client.connection = fake_conn + client.authenticated = True + + msg = EmailMessage() + msg["Subject"] = "Test" + msg.set_content("Test body") + + fake_conn.append_response = ("NO", [b"Mailbox does not exist"]) + + with pytest.raises(RequestException): + client.save_mail_to_folder(msg, cs.MAIL_FOLDER_NORMAL) + + def test_save_mail_to_folder_not_authenticated_raises(self): + """Test that save_mail_to_folder raises BugException if not authenticated.""" + from email.message import EmailMessage + client = make_client() + client.connection = None + + msg = EmailMessage() + msg["Subject"] = "Test" + msg.set_content("Test body") + + with pytest.raises(BugException): + client.save_mail_to_folder(msg, cs.MAIL_FOLDER_SENT) + + def test_save_mail_to_draft_folder_creates_if_missing(self): + """Test that save_mail_to_folder creates DRAFT folder if it doesn't exist.""" + from email.message import EmailMessage + fake_conn = FakeIMAPConnection() + client = authenticated_client(fake_conn) + + msg = EmailMessage() + msg["Subject"] = "Test" + msg.set_content("Test body") + + # Setup: first append call fails, second succeeds + append_responses = [ + ("NO", [b"Mailbox does not exist"]), + ("OK", [b""]) + ] + call_count = 0 + def append_response_side_effect(*args, **kwargs): + nonlocal call_count + result = append_responses[call_count % len(append_responses)] + call_count += 1 + return result + + with mock.patch.object(client, "_imap_create_folder") as mock_create: + with mock.patch.object(fake_conn, "append", side_effect=append_response_side_effect): + # MAIL_FOLDER_DRAFT is a special folder, not NORMAL + client.save_mail_to_folder(msg, cs.MAIL_FOLDER_DRAFT) + mock_create.assert_called_once()