From d32e94715aee2c818df612d020d5d78d0b0904c3 Mon Sep 17 00:00:00 2001 From: Christian Krakau-Louis Date: Tue, 1 Apr 2025 05:31:40 +0200 Subject: [PATCH] feat: enhance email upload functionality with recipient validation and SMTP error handling --- app/api.py | 2 +- app/tasks/send_to_all.py | 169 +++++++++++++++++++++------------ app/tasks/upload_to_dropbox.py | 31 +++--- app/tasks/upload_to_email.py | 111 ++++++++++++---------- 4 files changed, 190 insertions(+), 123 deletions(-) diff --git a/app/api.py b/app/api.py index a417631d..d3568bb0 100644 --- a/app/api.py +++ b/app/api.py @@ -231,7 +231,7 @@ def process_all_pdfs_in_workdir(): @router.post("/ui-upload") @require_login -async def ui_upload(file: UploadFile = File(...)): +async def ui_upload(request: Request, file: UploadFile = File(...)): """Endpoint to accept a user-uploaded file and enqueue it for processing.""" import uuid import os.path diff --git a/app/tasks/send_to_all.py b/app/tasks/send_to_all.py index 39a94993..f7462bf6 100644 --- a/app/tasks/send_to_all.py +++ b/app/tasks/send_to_all.py @@ -18,6 +18,55 @@ from app.celery_app import celery logger = logging.getLogger(__name__) +def _should_upload_to_dropbox(): + return (settings.dropbox_app_key and + settings.dropbox_app_secret and + settings.dropbox_refresh_token) + +def _should_upload_to_nextcloud(): + return (settings.nextcloud_upload_url and + settings.nextcloud_username and + settings.nextcloud_password) + +def _should_upload_to_paperless(): + return (settings.paperless_ngx_api_token and + settings.paperless_host) + +def _should_upload_to_google_drive(): + return settings.google_drive_credentials_json + +def _should_upload_to_webdav(): + return (settings.webdav_url and + settings.webdav_username and + settings.webdav_password) + +def _should_upload_to_ftp(): + return (settings.ftp_host and + settings.ftp_username and + settings.ftp_password) + +def _should_upload_to_sftp(): + return (settings.sftp_host and + settings.sftp_username and + (settings.sftp_password or settings.sftp_private_key)) + +def _should_upload_to_email(): + return (settings.email_host and + settings.email_username and + settings.email_password and + settings.email_default_recipient) + +def _should_upload_to_onedrive(): + return (settings.onedrive_client_id and + settings.onedrive_client_secret and + (settings.onedrive_refresh_token or + (settings.onedrive_tenant_id and settings.onedrive_tenant_id != "common"))) + +def _should_upload_to_s3(): + return (settings.s3_bucket_name and + settings.aws_access_key_id and + settings.aws_secret_access_key) + @celery.task(base=BaseTaskWithRetry) def send_to_all_destinations(file_path: str): """Distribute a file to all configured storage destinations.""" @@ -28,68 +77,66 @@ def send_to_all_destinations(file_path: str): logger.info(f"Sending {file_path} to all configured destinations") results = {} - # Send to Dropbox if configured - if settings.dropbox_app_key and settings.dropbox_app_secret and settings.dropbox_refresh_token: - logger.info(f"Queueing {file_path} for Dropbox upload") - task = upload_to_dropbox.delay(file_path) - results["dropbox_task_id"] = task.id + # Define service configurations + services = [ + { + "name": "dropbox", + "should_upload": _should_upload_to_dropbox, + "upload_func": upload_to_dropbox, + }, + { + "name": "nextcloud", + "should_upload": _should_upload_to_nextcloud, + "upload_func": upload_to_nextcloud, + }, + { + "name": "paperless", + "should_upload": _should_upload_to_paperless, + "upload_func": upload_to_paperless, + }, + { + "name": "google_drive", + "should_upload": _should_upload_to_google_drive, + "upload_func": upload_to_google_drive, + }, + { + "name": "webdav", + "should_upload": _should_upload_to_webdav, + "upload_func": upload_to_webdav, + }, + { + "name": "ftp", + "should_upload": _should_upload_to_ftp, + "upload_func": upload_to_ftp, + }, + { + "name": "sftp", + "should_upload": _should_upload_to_sftp, + "upload_func": upload_to_sftp, + }, + { + "name": "email", + "should_upload": _should_upload_to_email, + "upload_func": upload_to_email, + }, + { + "name": "onedrive", + "should_upload": _should_upload_to_onedrive, + "upload_func": upload_to_onedrive, + }, + { + "name": "s3", + "should_upload": _should_upload_to_s3, + "upload_func": upload_to_s3, + }, + ] - # Send to Nextcloud if configured - if settings.nextcloud_upload_url and settings.nextcloud_username and settings.nextcloud_password: - logger.info(f"Queueing {file_path} for Nextcloud upload") - task = upload_to_nextcloud.delay(file_path) - results["nextcloud_task_id"] = task.id - - # Send to Paperless if configured - if settings.paperless_ngx_api_token and settings.paperless_host: - logger.info(f"Queueing {file_path} for Paperless upload") - task = upload_to_paperless.delay(file_path) - results["paperless_task_id"] = task.id - - # Send to Google Drive if configured - if settings.google_drive_credentials_json: - logger.info(f"Queueing {file_path} for Google Drive upload") - task = upload_to_google_drive.delay(file_path) - results["google_drive_task_id"] = task.id - - # Send to WebDAV if configured - if settings.webdav_url and settings.webdav_username and settings.webdav_password: - logger.info(f"Queueing {file_path} for WebDAV upload") - task = upload_to_webdav.delay(file_path) - results["webdav_task_id"] = task.id - - # Send to FTP if configured - if settings.ftp_host and settings.ftp_username and settings.ftp_password: - logger.info(f"Queueing {file_path} for FTP upload") - task = upload_to_ftp.delay(file_path) - results["ftp_task_id"] = task.id - - # Send to SFTP if configured - if settings.sftp_host and settings.sftp_username and (settings.sftp_password or settings.sftp_private_key): - logger.info(f"Queueing {file_path} for SFTP upload") - task = upload_to_sftp.delay(file_path) - results["sftp_task_id"] = task.id - - # Send via email if configured - if settings.email_host and settings.email_username and settings.email_password and settings.email_default_recipient: - logger.info(f"Queueing {file_path} for email delivery") - task = upload_to_email.delay(file_path) - results["email_task_id"] = task.id - - # Send to OneDrive if configured - if settings.onedrive_client_id and settings.onedrive_client_secret and ( - settings.onedrive_refresh_token or - (settings.onedrive_tenant_id and settings.onedrive_tenant_id != "common") - ): - logger.info(f"Queueing {file_path} for OneDrive upload") - task = upload_to_onedrive.delay(file_path) - results["onedrive_task_id"] = task.id - - # Send to Amazon S3 if configured - if settings.s3_bucket_name and settings.aws_access_key_id and settings.aws_secret_access_key: - logger.info(f"Queueing {file_path} for S3 upload") - task = upload_to_s3.delay(file_path) - results["s3_task_id"] = task.id + # Process each service + for service in services: + if service["should_upload"](): + logger.info(f"Queueing {file_path} for {service['name']} upload") + task = service["upload_func"].delay(file_path) + results[f"{service['name']}_task_id"] = task.id return { "status": "Queued", diff --git a/app/tasks/upload_to_dropbox.py b/app/tasks/upload_to_dropbox.py index 0a13077a..24497a95 100644 --- a/app/tasks/upload_to_dropbox.py +++ b/app/tasks/upload_to_dropbox.py @@ -12,20 +12,30 @@ from app.utils.filename_utils import get_unique_filename, sanitize_filename, ext logger = logging.getLogger(__name__) +def _validate_dropbox_settings(): + """Validate that all required Dropbox settings are available.""" + missing = [] + + if not hasattr(settings, 'dropbox_refresh_token') or not settings.dropbox_refresh_token: + missing.append("refresh token") + + if not hasattr(settings, 'dropbox_app_key') or not settings.dropbox_app_key: + missing.append("app key") + + if not hasattr(settings, 'dropbox_app_secret') or not settings.dropbox_app_secret: + missing.append("app secret") + + if missing: + logger.error(f"Cannot refresh Dropbox token: Missing {', '.join(missing)}") + return False + + return True + def get_dropbox_access_token(): """Refresh the Dropbox access token using the stored refresh token from ENV.""" # Check if needed settings are available - if not hasattr(settings, 'dropbox_refresh_token') or not settings.dropbox_refresh_token: - logger.error("Cannot refresh Dropbox token: Missing refresh token") - return None - - if not hasattr(settings, 'dropbox_app_key') or not settings.dropbox_app_key: - logger.error("Cannot refresh Dropbox token: Missing app key") - return None - - if not hasattr(settings, 'dropbox_app_secret') or not settings.dropbox_app_secret: - logger.error("Cannot refresh Dropbox token: Missing app secret") + if not _validate_dropbox_settings(): return None token_url = "https://api.dropbox.com/oauth2/token" @@ -64,7 +74,6 @@ def upload_to_dropbox(file_path: str): return {"status": "Skipped", "reason": "Dropbox settings not configured"} filename = os.path.basename(file_path) - sanitized_filename = sanitize_filename(filename) try: # Get access token from refresh token diff --git a/app/tasks/upload_to_email.py b/app/tasks/upload_to_email.py index 2b85d4d2..e2656461 100644 --- a/app/tasks/upload_to_email.py +++ b/app/tasks/upload_to_email.py @@ -117,6 +117,48 @@ def attach_logo(msg): logger.warning(f"Error attaching logo: {str(e)}") return False +def _prepare_recipients(recipients): + """Helper function to prepare email recipients list.""" + if not recipients: + if not settings.email_default_recipient: + error_msg = "No recipients specified and no default recipient configured" + logger.error(error_msg) + return None, error_msg + return [settings.email_default_recipient], None + elif isinstance(recipients, str): + return [recipients], None # Convert single email to list + return recipients, None + +def _send_email_with_smtp(msg, filename, recipients): + """Helper function to handle SMTP connection and sending.""" + try: + # First try to resolve the hostname + socket.gethostbyname(settings.email_host) + + # Connect to the SMTP server + with smtplib.SMTP(settings.email_host, settings.email_port, timeout=30) as server: + # Use TLS if specified + if settings.email_use_tls: + server.starttls() + + # Login if credentials are provided + if settings.email_username and settings.email_password: + server.login(settings.email_username, settings.email_password) + + # Send the email + server.send_message(msg) + + logger.info(f"Successfully sent {filename} via email to {', '.join(recipients)}") + return None + except socket.gaierror as e: + error_msg = f"Failed to resolve email host: {settings.email_host} - {str(e)}" + logger.error(error_msg) + return {"status": "Failed", "reason": error_msg, "error": str(e)} + except (ConnectionRefusedError, TimeoutError) as e: + error_msg = f"Connection error to SMTP server {settings.email_host}:{settings.email_port} - {str(e)}" + logger.error(error_msg) + return {"status": "Failed", "reason": error_msg, "error": str(e)} + @celery.task(base=BaseTaskWithRetry) def upload_to_email(file_path: str, recipients=None, subject=None, message=None, template_name="default.html", include_metadata=True): """ @@ -139,19 +181,13 @@ def upload_to_email(file_path: str, recipients=None, subject=None, message=None, logger.debug(f"Email config - Host: {settings.email_host}, Port: {settings.email_port}, " f"Username: {settings.email_username}, TLS: {settings.email_use_tls}") - # Use provided recipients or fall back to default - if not recipients: - if not settings.email_default_recipient: - error_msg = "No recipients specified and no default recipient configured" - logger.error(error_msg) - return {"status": "Skipped", "reason": error_msg} - recipients = [settings.email_default_recipient] - elif isinstance(recipients, str): - recipients = [recipients] # Convert single email to list + # Process recipients + recipients, error = _prepare_recipients(recipients) + if error: + return {"status": "Skipped", "reason": error} # Use provided subject or create default - if not subject: - subject = f"DocuNova Document: {filename}" + subject = subject or f"DocuNova Document: {filename}" # Extract document metadata if available metadata = {} @@ -160,7 +196,7 @@ def upload_to_email(file_path: str, recipients=None, subject=None, message=None, try: # Create the email - msg = MIMEMultipart('related') # Changed to 'related' to properly handle inline images + msg = MIMEMultipart('related') msg['From'] = settings.email_sender or settings.email_username msg['To'] = ", ".join(recipients) msg['Subject'] = subject @@ -198,45 +234,20 @@ def upload_to_email(file_path: str, recipients=None, subject=None, message=None, attachment.add_header('Content-Disposition', f'attachment; filename="{filename}"') msg.attach(attachment) - try: - # First try to resolve the hostname - socket.gethostbyname(settings.email_host) - - # Connect to the SMTP server - with smtplib.SMTP(settings.email_host, settings.email_port, timeout=30) as server: - # Use TLS if specified - if settings.email_use_tls: - server.starttls() - - # Login if credentials are provided - if settings.email_username and settings.email_password: - server.login(settings.email_username, settings.email_password) - - # Send the email - server.send_message(msg) - - logger.info(f"Successfully sent {filename} via email to {', '.join(recipients)}") - return { - "status": "Completed", - "file": file_path, - "recipients": recipients, - "subject": subject, - "metadata_included": bool(metadata), - "logo_included": has_logo - } - except socket.gaierror as e: - error_msg = f"Failed to resolve email host: {settings.email_host} - {str(e)}" - logger.error(error_msg) - return {"status": "Failed", "reason": error_msg, "error": str(e)} - except ConnectionRefusedError as e: - error_msg = f"Connection refused to SMTP server {settings.email_host}:{settings.email_port} - {str(e)}" - logger.error(error_msg) - return {"status": "Failed", "reason": error_msg, "error": str(e)} - except TimeoutError as e: - error_msg = f"Connection timeout to SMTP server {settings.email_host}:{settings.email_port} - {str(e)}" - logger.error(error_msg) - return {"status": "Failed", "reason": error_msg, "error": str(e)} + # Send the email through SMTP + error_result = _send_email_with_smtp(msg, filename, recipients) + if error_result: + return error_result + return { + "status": "Completed", + "file": file_path, + "recipients": recipients, + "subject": subject, + "metadata_included": bool(metadata), + "logo_included": has_logo + } + except Exception as e: error_msg = f"Failed to send {filename} via email: {str(e)}" logger.error(error_msg)