diff --git a/.env.example b/.env.example index 8476abc..b6b69b5 100644 --- a/.env.example +++ b/.env.example @@ -9,6 +9,9 @@ # LETTERFEED_IMAP_SERVER= # LETTERFEED_IMAP_USERNAME= # LETTERFEED_IMAP_PASSWORD= +# The TLS certificate of the IMAP server is verified. Only set this to true for a server +# with a self-signed certificate; this disables certificate verification. +# LETTERFEED_IMAP_TLS_INSECURE=false # Email processing settings # LETTERFEED_SEARCH_FOLDER=INBOX # The folder in which to search for new emails diff --git a/README.md b/README.md index b6c54e0..ecd9950 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ It periodically scans your email inbox via IMAP for new emails from the senders ### Prerequisites -1. An existing mailbox with IMAP over SSL on port 993. +1. An existing mailbox with IMAP over SSL on port 993. The server's TLS certificate is verified; for a server with a self-signed certificate, set `LETTERFEED_IMAP_TLS_INSECURE=true` (this disables certificate verification). 2. Docker and Docker Compose installed on your system. ### Installation diff --git a/backend/app/core/config.py b/backend/app/core/config.py index 18d5afa..6f27b92 100644 --- a/backend/app/core/config.py +++ b/backend/app/core/config.py @@ -27,6 +27,7 @@ class Settings(BaseSettings): imap_server: str = "" imap_username: str = "" imap_password: str = "" + imap_tls_insecure: bool = False search_folder: str = "INBOX" move_to_folder: str | None = None mark_as_read: bool = False diff --git a/backend/app/core/imap.py b/backend/app/core/imap.py index 4c0c7cd..4cd223a 100644 --- a/backend/app/core/imap.py +++ b/backend/app/core/imap.py @@ -1,5 +1,7 @@ import imaplib +import ssl +from app.core.config import settings as env_settings from app.core.logging import get_logger """IMAP utility functions for connecting to mail servers and fetching folders.""" @@ -7,6 +9,32 @@ logger = get_logger(__name__) +def imap_ssl_context() -> ssl.SSLContext: + """Return the TLS context used for IMAP connections. + + The server certificate and hostname are verified against the system CA store, + unless LETTERFEED_IMAP_TLS_INSECURE is enabled. + """ + if env_settings.imap_tls_insecure: + logger.warning( + "LETTERFEED_IMAP_TLS_INSECURE is enabled: the IMAP server certificate is not verified" + ) + return ssl._create_unverified_context() + return ssl.create_default_context() + + +def connect_imap(server: str) -> imaplib.IMAP4_SSL: + """Open an IMAP over TLS connection with certificate verification.""" + try: + return imaplib.IMAP4_SSL(server, ssl_context=imap_ssl_context()) + except ssl.SSLCertVerificationError as e: + reason = str(e.verify_message or e).rstrip(".") + raise ConnectionError( + f"TLS certificate verification failed for {server}: {reason}. " + "If the server uses a self-signed certificate, set LETTERFEED_IMAP_TLS_INSECURE=true." + ) from e + + def send_client_id(mail: imaplib.IMAP4_SSL) -> None: """Send IMAP client identity for providers that require RFC 2971 ID.""" imaplib.Commands.setdefault("ID", ("AUTH", "SELECTED")) @@ -22,7 +50,7 @@ def _test_imap_connection(server, username, password): """Test the IMAP connection with the given credentials.""" logger.info(f"Testing IMAP connection to {server} for user {username}") try: - mail = imaplib.IMAP4_SSL(server) + mail = connect_imap(server) mail.login(username, password) send_client_id(mail) mail.logout() @@ -37,7 +65,7 @@ def get_folders(server, username, password): """Fetch a list of IMAP folders from the mail server.""" logger.info(f"Fetching IMAP folders from {server} for user {username}") try: - mail = imaplib.IMAP4_SSL(server) + mail = connect_imap(server) mail.login(username, password) send_client_id(mail) status, folders = mail.list() diff --git a/backend/app/core/scheduler.py b/backend/app/core/scheduler.py index 0ca6bae..3503b10 100644 --- a/backend/app/core/scheduler.py +++ b/backend/app/core/scheduler.py @@ -5,7 +5,7 @@ from app.core.database import SessionLocal from app.core.logging import get_logger from app.crud.settings import get_settings -from app.services.email_processor import process_emails +from app.services.email_processor import EmailProcessingError, process_emails """Scheduler for background tasks like email processing.""" @@ -19,6 +19,8 @@ def job(): try: process_emails(db) logger.info("Scheduler job finished: process_emails") + except EmailProcessingError as e: + logger.warning(f"Scheduled email processing failed: {e}") except Exception as e: logger.error(f"Error in scheduled job process_emails: {e}", exc_info=True) finally: diff --git a/backend/app/routers/imap.py b/backend/app/routers/imap.py index 1592b7c..83e737c 100644 --- a/backend/app/routers/imap.py +++ b/backend/app/routers/imap.py @@ -8,7 +8,7 @@ from app.core.logging import get_logger from app.crud.settings import create_or_update_settings, get_settings from app.schemas.settings import Settings, SettingsCreate -from app.services.email_processor import process_emails +from app.services.email_processor import EmailProcessingError, process_emails logger = get_logger(__name__) router = APIRouter() @@ -81,6 +81,9 @@ def trigger_email_processing(db: Session = Depends(get_db)): try: process_emails(db) return {"message": "Email processing triggered successfully."} + except EmailProcessingError as e: + logger.warning(f"Email processing failed: {e}") + raise HTTPException(status_code=400, detail=str(e)) except Exception as e: logger.error(f"Error triggering email processing: {e}", exc_info=True) raise HTTPException(status_code=500, detail=str(e)) diff --git a/backend/app/services/email_processor.py b/backend/app/services/email_processor.py index cf72eb1..3b9e328 100644 --- a/backend/app/services/email_processor.py +++ b/backend/app/services/email_processor.py @@ -10,7 +10,7 @@ from readability import Document from sqlalchemy.orm import Session -from app.core.imap import send_client_id +from app.core.imap import connect_imap, send_client_id from app.core.logging import get_logger from app.crud.entries import create_entry, get_entry_by_message_id from app.crud.newsletters import create_newsletter, get_newsletters @@ -23,6 +23,17 @@ logger = get_logger(__name__) +class EmailProcessingError(Exception): + """Raised when emails could not be fetched from the IMAP server.""" + + +def _error_message(error: Exception) -> str: + """Return a readable message for an IMAP or connection error.""" + if error.args and isinstance(error.args[0], bytes): + return error.args[0].decode("utf-8", "replace") + return str(error) or type(error).__name__ + + def _is_configured(settings: Settings | None) -> bool: """Check if IMAP settings are configured.""" if ( @@ -36,27 +47,32 @@ def _is_configured(settings: Settings | None) -> bool: return True -def _connect_to_imap( - settings: Settings, search_folder: str -) -> imaplib.IMAP4_SSL | None: - """Connect to the IMAP server and select the mailbox.""" +def _connect_to_imap(settings: Settings, search_folder: str) -> imaplib.IMAP4_SSL: + """Connect to the IMAP server and select the mailbox. + + Raises: + EmailProcessingError: If the connection, the login or selecting the + mailbox fails. + """ try: logger.info(f"Connecting to IMAP server: {settings.imap_server}") - mail = imaplib.IMAP4_SSL(settings.imap_server) + mail = connect_imap(settings.imap_server) mail.login(settings.imap_username, settings.imap_password) send_client_id(mail) status, messages = mail.select(search_folder) - if status != "OK": - logger.error( - f"Failed to select mailbox: {search_folder}, status: {status}, messages: {messages}" - ) - mail.logout() - return None - logger.info(f"Selected mailbox: {search_folder}") - return mail except Exception as e: logger.error(f"Failed to connect to IMAP server: {e}", exc_info=True) - return None + raise EmailProcessingError( + f"Could not connect to IMAP server: {_error_message(e)}" + ) from e + if status != "OK": + logger.error( + f"Failed to select mailbox: {search_folder}, status: {status}, messages: {messages}" + ) + mail.logout() + raise EmailProcessingError(f"Could not select IMAP folder '{search_folder}'") + logger.info(f"Selected mailbox: {search_folder}") + return mail def _fetch_unread_email_ids(mail: imaplib.IMAP4_SSL) -> list[str]: @@ -247,7 +263,12 @@ def _process_single_email( def process_emails(db: Session) -> None: - """Process unread emails, add them as entries, and manage newsletters.""" + """Process unread emails, add them as entries, and manage newsletters. + + Raises: + EmailProcessingError: If one or more folders could not be processed. + All other folders are still processed. + """ logger.info("Starting email processing...") settings = get_settings(db, with_password=True) if not _is_configured(settings): @@ -268,6 +289,7 @@ def process_emails(db: Session) -> None: if settings.auto_add_new_senders and settings.search_folder not in folder_groups: folder_groups[settings.search_folder] = [] + errors: list[str] = [] for search_folder, newsletters_in_folder in folder_groups.items(): logger.info( f"Processing folder '{search_folder}' for {len(newsletters_in_folder)} newsletters." @@ -276,11 +298,11 @@ def process_emails(db: Session) -> None: sender.email: nl for nl in newsletters_in_folder for sender in nl.senders } - mail = _connect_to_imap(settings, search_folder) - if not mail: - logger.warning( - f"Skipping folder '{search_folder}' due to connection issue." - ) + try: + mail = _connect_to_imap(settings, search_folder) + except EmailProcessingError as e: + logger.warning(f"Skipping folder '{search_folder}': {e}") + errors.append(str(e)) continue try: @@ -307,7 +329,15 @@ def process_emails(db: Session) -> None: f"Error processing emails in folder '{search_folder}': {e}", exc_info=True, ) + errors.append( + f"Error processing folder '{search_folder}': {_error_message(e)}" + ) finally: mail.logout() + if errors: + # Report every distinct problem once, e.g. a connection error that + # occurred for several folders. + raise EmailProcessingError("; ".join(dict.fromkeys(errors))) + logger.info("Email processing finished successfully.") diff --git a/backend/app/tests/test_core.py b/backend/app/tests/test_core.py index 27ad044..8a75f8a 100644 --- a/backend/app/tests/test_core.py +++ b/backend/app/tests/test_core.py @@ -1,14 +1,20 @@ +import ssl from datetime import datetime from unittest.mock import ANY, MagicMock, patch from sqlalchemy.orm import Session -from app.core.imap import _test_imap_connection, get_folders, send_client_id +from app.core.imap import ( + _test_imap_connection, + connect_imap, + get_folders, + send_client_id, +) from app.crud.newsletters import create_newsletter from app.crud.settings import create_or_update_settings from app.schemas.newsletters import NewsletterCreate from app.schemas.settings import SettingsCreate -from app.services.email_processor import process_emails +from app.services.email_processor import EmailProcessingError, process_emails @patch("app.core.imap.imaplib.IMAP4_SSL") @@ -43,6 +49,40 @@ def test_get_folders(mock_imap): assert folders == ["INBOX", "Processed"] +@patch("app.core.imap.imaplib.IMAP4_SSL") +def test_connect_imap_verifies_certificate(mock_imap): + """Test that IMAP connections verify the server certificate and hostname.""" + connect_imap("imap.test.com") + context = mock_imap.call_args.kwargs["ssl_context"] + assert mock_imap.call_args.args == ("imap.test.com",) + assert context.verify_mode == ssl.CERT_REQUIRED + assert context.check_hostname + + +@patch("app.core.imap.env_settings") +@patch("app.core.imap.imaplib.IMAP4_SSL") +def test_connect_imap_insecure_opt_out(mock_imap, mock_env_settings): + """Test that certificate verification can be disabled explicitly.""" + mock_env_settings.imap_tls_insecure = True + connect_imap("imap.test.com") + context = mock_imap.call_args.kwargs["ssl_context"] + assert context.verify_mode == ssl.CERT_NONE + assert not context.check_hostname + + +@patch("app.core.imap.imaplib.IMAP4_SSL") +def test_test_imap_connection_certificate_error(mock_imap): + """Test that a certificate verification error is reported.""" + error = ssl.SSLCertVerificationError("certificate verify failed") + error.verify_message = "self-signed certificate" + mock_imap.side_effect = error + success, message = _test_imap_connection("imap.test.com", "user", "pass") + assert not success + assert "TLS certificate verification failed for imap.test.com" in message + assert "self-signed certificate" in message + assert "LETTERFEED_IMAP_TLS_INSECURE" in message + + def test_send_client_id_registers_and_sends_imap_id_command(): """Test sending IMAP client identity for providers that require it.""" mock_mail = MagicMock() @@ -160,6 +200,23 @@ def test_scheduler_job(mock_process_emails, mock_session_local, db_session: Sess mock_process_emails.assert_called_once_with(db_session) +@patch("app.core.scheduler.logger") +@patch("app.core.scheduler.SessionLocal") +@patch("app.core.scheduler.process_emails") +def test_scheduler_job_email_processing_error( + mock_process_emails, mock_session_local, mock_logger, db_session: Session +): + """Test that the scheduler job logs email processing errors.""" + mock_session_local.return_value = db_session + mock_process_emails.side_effect = EmailProcessingError("Could not connect") + from app.core.scheduler import job + + job() + mock_logger.warning.assert_called_once_with( + "Scheduled email processing failed: Could not connect" + ) + + @patch("app.services.email_processor.imaplib.IMAP4_SSL") def test_process_emails_auto_add_sender(mock_imap, db_session: Session): """Test processing emails with auto add sender enabled.""" diff --git a/backend/app/tests/test_email_processor.py b/backend/app/tests/test_email_processor.py index d1fd1de..ce293d5 100644 --- a/backend/app/tests/test_email_processor.py +++ b/backend/app/tests/test_email_processor.py @@ -2,6 +2,7 @@ from email.message import Message from unittest.mock import MagicMock, patch +import pytest from sqlalchemy.orm import Session from app.crud.newsletters import create_newsletter @@ -9,7 +10,11 @@ from app.models.newsletters import Newsletter from app.schemas.newsletters import NewsletterCreate from app.schemas.settings import Settings, SettingsCreate -from app.services.email_processor import _process_single_email, process_emails +from app.services.email_processor import ( + EmailProcessingError, + _process_single_email, + process_emails, +) def _setup_test_email_processing( @@ -107,7 +112,7 @@ def test_process_emails_uses_newsletter_search_folder( create_newsletter(db_session, newsletter_data) # Mock the return of _connect_to_imap to avoid a real IMAP connection - mock_connect_to_imap.return_value = None + mock_connect_to_imap.return_value.search.return_value = ("OK", [b""]) # 2. ACT process_emails(db_session) @@ -141,7 +146,7 @@ def test_process_emails_uses_global_search_folder( ) create_newsletter(db_session, newsletter_data) - mock_connect_to_imap.return_value = None + mock_connect_to_imap.return_value.search.return_value = ("OK", [b""]) # 2. ACT process_emails(db_session) @@ -278,3 +283,80 @@ def test_process_single_email_with_null_bytes_in_body(db_session: Session): # The body should be the original (but decoded) body, since extraction failed # Note: _get_email_body will decode the payload. assert "Hello\x00 World" in entry_create_arg.body + + +def _create_settings_and_newsletters(db_session: Session, folders: list[str]) -> None: + """Create IMAP settings and one newsletter per search folder.""" + create_or_update_settings( + db_session, + SettingsCreate( + imap_server="imap.test.com", + imap_username="user@test.com", + imap_password="secret-password", + search_folder="INBOX", + ), + ) + for i, folder in enumerate(folders): + create_newsletter( + db_session, + NewsletterCreate( + name=f"Newsletter {i}", + sender_emails=[f"sender{i}@example.com"], + search_folder=folder, + ), + ) + + +@patch("app.services.email_processor.connect_imap") +def test_process_emails_reports_connection_error(mock_connect_imap, db_session): + """Test that a connection error is raised once, without the password.""" + _create_settings_and_newsletters(db_session, ["INBOX", "Newsletters"]) + mock_connect_imap.side_effect = ConnectionError( + "TLS certificate verification failed for imap.test.com: self-signed certificate" + ) + + with pytest.raises(EmailProcessingError) as exc_info: + process_emails(db_session) + + message = str(exc_info.value) + assert message == ( + "Could not connect to IMAP server: TLS certificate verification failed " + "for imap.test.com: self-signed certificate" + ) + assert "secret-password" not in message + assert mock_connect_imap.call_count == 2 + + +@patch("app.services.email_processor.connect_imap") +def test_process_emails_reports_login_error(mock_connect_imap, db_session): + """Test that an IMAP login error is reported as readable text.""" + _create_settings_and_newsletters(db_session, ["INBOX"]) + mock_connect_imap.return_value.login.side_effect = imaplib.IMAP4.error( + b"[AUTHENTICATIONFAILED] Invalid credentials" + ) + + with pytest.raises(EmailProcessingError) as exc_info: + process_emails(db_session) + + assert str(exc_info.value) == ( + "Could not connect to IMAP server: [AUTHENTICATIONFAILED] Invalid credentials" + ) + + +@patch("app.services.email_processor.connect_imap") +def test_process_emails_reports_missing_folder_and_continues( + mock_connect_imap, db_session +): + """Test that a failing folder is reported and the other folders are processed.""" + _create_settings_and_newsletters(db_session, ["Missing", "INBOX"]) + mock_mail = mock_connect_imap.return_value + mock_mail.select.side_effect = lambda folder: ( + ("NO", [b"Mailbox does not exist"]) if folder == "Missing" else ("OK", [b"0"]) + ) + mock_mail.search.return_value = ("OK", [b""]) + + with pytest.raises(EmailProcessingError) as exc_info: + process_emails(db_session) + + assert str(exc_info.value) == "Could not select IMAP folder 'Missing'" + mock_mail.search.assert_called_once() diff --git a/backend/app/tests/test_routers.py b/backend/app/tests/test_routers.py index 0778c42..673b205 100644 --- a/backend/app/tests/test_routers.py +++ b/backend/app/tests/test_routers.py @@ -6,6 +6,7 @@ from app.crud.settings import create_or_update_settings from app.schemas.settings import SettingsCreate +from app.services.email_processor import EmailProcessingError def test_health_check(client: TestClient): @@ -107,6 +108,19 @@ def test_trigger_email_processing(mock_process_emails, client: TestClient): mock_process_emails.assert_called_once() +@patch("app.routers.imap.process_emails") +def test_trigger_email_processing_error(mock_process_emails, client: TestClient): + """Test that an email processing error is returned to the client.""" + mock_process_emails.side_effect = EmailProcessingError( + "Could not connect to IMAP server: [AUTHENTICATIONFAILED] Invalid credentials" + ) + response = client.post("/imap/process") + assert response.status_code == 400 + assert response.json() == { + "detail": "Could not connect to IMAP server: [AUTHENTICATIONFAILED] Invalid credentials" + } + + def test_create_newsletter(client: TestClient): """Test creating a newsletter.""" unique_email = f"newsletter_{uuid.uuid4()}@example.com"