From c31b72810e5fef29020264c88db271a65567bfcb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Mar 2026 11:11:21 +0000 Subject: [PATCH] fix(merge): resolve tests/test_auth.py conflict keeping all tests from both branches Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> --- .env.demo | 16 +++++- BUILD_DATE | 2 +- CHANGELOG.md | 25 +++++++++ GIT_SHA | 2 +- RUNTIME_INFO | 12 ++--- VERSION | 2 +- app/auth.py | 13 +++++ app/config.py | 11 +++- app/tasks/finalize_document_storage.py | 7 --- app/tasks/send_to_all.py | 5 +- app/tasks/upload_to_email.py | 30 +++++------ app/templates/email/default.html | 63 ++++++++++++++++++++++ app/utils/config_validator/providers.py | 18 +++---- app/utils/config_validator/validators.py | 10 ++-- app/utils/settings_service.py | 61 +++++++++++++++++++++- docs/ConfigurationGuide.md | 35 +++++++++++-- tests/test_auth.py | 55 ++++++++++++++++++++ tests/test_auth_module.py | 27 ++++++++++ tests/test_config_validators.py | 14 ++--- tests/test_coverage_remaining_gaps.py | 7 +++ tests/test_finalize_storage.py | 66 ++++++++++++++++++++++++ tests/test_send_to_all.py | 8 +-- tests/test_upload_email.py | 50 +++++++++--------- tests/test_upload_tasks.py | 14 ++--- 24 files changed, 455 insertions(+), 98 deletions(-) create mode 100644 app/templates/email/default.html diff --git a/.env.demo b/.env.demo index 55eff07d..a2f567b5 100644 --- a/.env.demo +++ b/.env.demo @@ -197,14 +197,26 @@ OPENAI_MODEL=gpt-4o-mini # AI_MODEL=gpt-4o # deployment name in Azure # Azure Document Intelligence (OCR – separate from AI provider above) -# **Email Settings** +# **Email Settings (shared SMTP – password reset, verification, and system notifications)** EMAIL_HOST=smtp.example.com EMAIL_PORT=587 EMAIL_USERNAME=docuelevate@example.com EMAIL_PASSWORD=your_secure_email_password EMAIL_USE_TLS=True EMAIL_SENDER=DocuElevate System -EMAIL_DEFAULT_RECIPIENT=recipient@example.com +# EMAIL_DEFAULT_RECIPIENT is not used for document delivery (see DEST_EMAIL_* below) + +# **Email Destination Settings (dedicated SMTP for document delivery)** +# These settings are intentionally separate from the shared EMAIL_* settings above. +# Configuring EMAIL_HOST for password reset / notifications does NOT automatically +# enable the email destination – you must set DEST_EMAIL_HOST to activate it. +DEST_EMAIL_HOST=smtp.example.com +DEST_EMAIL_PORT=587 +DEST_EMAIL_USERNAME=docuelevate@example.com +DEST_EMAIL_PASSWORD=your_secure_email_password +DEST_EMAIL_USE_TLS=True +DEST_EMAIL_SENDER=DocuElevate Delivery +DEST_EMAIL_DEFAULT_RECIPIENT=recipient@example.com # **Watch Folder Ingestion** # DocuElevate can automatically monitor directories (local, FTP, SFTP, and cloud providers) for new files. diff --git a/BUILD_DATE b/BUILD_DATE index fc8a0076..98bb6ac5 100644 --- a/BUILD_DATE +++ b/BUILD_DATE @@ -1 +1 @@ -2026-03-08T09:05:40Z +2026-03-08T10:58:56Z diff --git a/CHANGELOG.md b/CHANGELOG.md index 14e7b27d..6778a21b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,31 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 +## v0.90.3 (2026-03-08) + +### Bug Fixes + +- **email**: Create missing email template and decouple email destination settings + ([`58c9b5d`](https://github.com/christianlouis/DocuElevate/commit/58c9b5d7f01941949db71b2816678ac6464d9d0f)) + + +## v0.90.2 (2026-03-08) + +### Bug Fixes + +- **tasks**: Remove erroneous in_progress log that regressed finalize_document_storage status when + PDF/A archival is enabled + ([`ff1310c`](https://github.com/christianlouis/DocuElevate/commit/ff1310c23ed6080f95957bf19fdb7cc179d10ecb)) + + +## v0.90.1 (2026-03-08) + +### Bug Fixes + +- **auth**: Return 401 for API paths in require_login to prevent wrong post-login redirect + ([`3aa5364`](https://github.com/christianlouis/DocuElevate/commit/3aa5364e0ca3eaceb37616bb9b3a9a55fc08b223)) + + ## v0.90.0 (2026-03-08) ### Features diff --git a/GIT_SHA b/GIT_SHA index df92da07..ac40d760 100644 --- a/GIT_SHA +++ b/GIT_SHA @@ -1 +1 @@ -bd9da65 +da47283 diff --git a/RUNTIME_INFO b/RUNTIME_INFO index 6eba300d..2a136b0a 100644 --- a/RUNTIME_INFO +++ b/RUNTIME_INFO @@ -1,10 +1,10 @@ DocuElevate Build Information ============================== -Version: 0.90.0 -Build Date: 2026-03-08T09:05:40Z -Git Commit: bd9da655117ef54300066c7354952dc9f12bbd9b -Git Short SHA: bd9da65 +Version: 0.90.3 +Build Date: 2026-03-08T10:58:56Z +Git Commit: da47283e0afb48e2992c7698b5ff74028c83d55e +Git Short SHA: da47283 Git Branch: main -Commit Date: 2026-03-08T10:05:10+01:00 -Build Timestamp: 2026-03-08T09:05:40Z +Commit Date: 2026-03-08T11:58:40+01:00 +Build Timestamp: 2026-03-08T10:58:56Z ============================== diff --git a/VERSION b/VERSION index ae02209b..c7709f43 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.90.0 +0.90.3 diff --git a/app/auth.py b/app/auth.py index 0e39c95b..5852a6dd 100644 --- a/app/auth.py +++ b/app/auth.py @@ -3,9 +3,11 @@ import inspect import logging import pathlib from functools import wraps +from urllib.parse import urlparse from authlib.integrations.starlette_client import OAuth from fastapi import APIRouter, Depends, Request, status +from fastapi.responses import JSONResponse from fastapi.templating import Jinja2Templates from sqlalchemy.orm import Session from starlette.responses import RedirectResponse @@ -81,6 +83,17 @@ def require_login(func): @wraps(func) async def wrapper(request: Request, *args, **kwargs): if not request.session.get("user"): + # For API endpoints return 401 instead of storing the URL in the session + # and redirecting to /login. Without this guard, the /api/auth/whoami + # probe issued by common.js on every page load would overwrite + # redirect_after_login with the API URL, causing the post-login redirect + # to land on a JSON endpoint rather than the original page. + url_path = urlparse(str(request.url)).path + if url_path.startswith("/api/"): + return JSONResponse( + status_code=status.HTTP_401_UNAUTHORIZED, + content={"error": "Not authenticated"}, + ) request.session["redirect_after_login"] = str(request.url) return RedirectResponse(url="/login", status_code=status.HTTP_302_FOUND) # Pass request as a keyword argument so that endpoints whose first diff --git a/app/config.py b/app/config.py index bb371913..8c9b4a9c 100644 --- a/app/config.py +++ b/app/config.py @@ -432,7 +432,7 @@ class Settings(BaseSettings): # In development/testing, set to True to disable verification (not recommended) sftp_disable_host_key_verification: bool = False # Default enforces host key verification - # Email settings + # Email settings (shared SMTP – used for password reset, verification emails, etc.) email_host: Optional[str] = None email_port: Optional[int] = 587 email_username: Optional[str] = None @@ -441,6 +441,15 @@ class Settings(BaseSettings): email_sender: Optional[str] = None # From address, defaults to email_username if not set email_default_recipient: Optional[str] = None + # Email destination settings (dedicated SMTP for document delivery – decoupled from shared email above) + dest_email_host: Optional[str] = None + dest_email_port: Optional[int] = 587 + dest_email_username: Optional[str] = None + dest_email_password: Optional[str] = None + dest_email_use_tls: bool = True + dest_email_sender: Optional[str] = None # From address for delivered documents + dest_email_default_recipient: Optional[str] = None # Fallback recipient for document delivery + # OneDrive settings onedrive_client_id: Optional[str] = None onedrive_client_secret: Optional[str] = None diff --git a/app/tasks/finalize_document_storage.py b/app/tasks/finalize_document_storage.py index e98de698..b2a7d919 100644 --- a/app/tasks/finalize_document_storage.py +++ b/app/tasks/finalize_document_storage.py @@ -80,13 +80,6 @@ def finalize_document_storage(self, original_file: str, processed_file: str, met from app.tasks.convert_to_pdfa import convert_to_pdfa logger.info(f"[{task_id}] PDF/A conversion enabled, queueing archival conversion") - log_task_progress( - task_id, - "finalize_document_storage", - "in_progress", - "Queueing PDF/A archival conversion", - file_id=file_id, - ) convert_to_pdfa.delay(file_id) except Exception as e: logger.warning(f"[{task_id}] Could not queue PDF/A conversion: {e}") diff --git a/app/tasks/send_to_all.py b/app/tasks/send_to_all.py index a0f7d983..f4d207c2 100644 --- a/app/tasks/send_to_all.py +++ b/app/tasks/send_to_all.py @@ -64,7 +64,10 @@ def _should_upload_to_sftp(): def _should_upload_to_email(): return bool( - settings.email_host and settings.email_username and settings.email_password and settings.email_default_recipient + settings.dest_email_host + and settings.dest_email_username + and settings.dest_email_password + and settings.dest_email_default_recipient ) diff --git a/app/tasks/upload_to_email.py b/app/tasks/upload_to_email.py index bd4580eb..8b9c8220 100644 --- a/app/tasks/upload_to_email.py +++ b/app/tasks/upload_to_email.py @@ -125,11 +125,11 @@ def attach_logo(msg): def _prepare_recipients(recipients): """Helper function to prepare email recipients list.""" if not recipients: - if not settings.email_default_recipient: + if not settings.dest_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 + return [settings.dest_email_default_recipient], None elif isinstance(recipients, str): return [recipients], None # Convert single email to list return recipients, None @@ -139,17 +139,17 @@ 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) + socket.gethostbyname(settings.dest_email_host) # Connect to the SMTP server - with smtplib.SMTP(settings.email_host, settings.email_port, timeout=30) as server: + with smtplib.SMTP(settings.dest_email_host, settings.dest_email_port, timeout=30) as server: # Use TLS if specified - if settings.email_use_tls: + if settings.dest_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) + if settings.dest_email_username and settings.dest_email_password: + server.login(settings.dest_email_username, settings.dest_email_password) # Send the email server.send_message(msg) @@ -157,11 +157,11 @@ def _send_email_with_smtp(msg, filename, recipients): 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)}" + error_msg = f"Failed to resolve email host: {settings.dest_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)}" + error_msg = f"Connection error to SMTP server {settings.dest_email_host}:{settings.dest_email_port} - {str(e)}" logger.error(error_msg) return {"status": "Failed", "reason": error_msg, "error": str(e)} @@ -205,17 +205,17 @@ def upload_to_email( # Extract filename filename = os.path.basename(file_path) - # Check if email settings are configured - if not settings.email_host: - error_msg = "Email host is not configured" + # Check if email destination settings are configured + if not settings.dest_email_host: + error_msg = "Email destination host is not configured (DEST_EMAIL_HOST)" logger.error(f"[{task_id}] {error_msg}") log_task_progress(task_id, "upload_to_email", "skipped", error_msg, file_id=file_id) return {"status": "Skipped", "reason": error_msg} # Log email configuration for debugging logger.debug( - f"[{task_id}] Email config - Host: {settings.email_host}, Port: {settings.email_port}, " - f"Username: {settings.email_username}, TLS: {settings.email_use_tls}" + f"[{task_id}] Email destination config - Host: {settings.dest_email_host}, Port: {settings.dest_email_port}, " + f"Username: {settings.dest_email_username}, TLS: {settings.dest_email_use_tls}" ) # Process recipients @@ -236,7 +236,7 @@ def upload_to_email( try: # Create the email msg = MIMEMultipart("related") - msg["From"] = settings.email_sender or settings.email_username + msg["From"] = settings.dest_email_sender or settings.dest_email_username msg["To"] = ", ".join(recipients) msg["Subject"] = subject diff --git a/app/templates/email/default.html b/app/templates/email/default.html new file mode 100644 index 00000000..f247d574 --- /dev/null +++ b/app/templates/email/default.html @@ -0,0 +1,63 @@ + + + + + + {{ filename }} – DocuElevate + + + +
+
+ {% if has_logo %} + {{ app_name }} logo + {% endif %} +

Document Delivery

+
+
+

{{ message }}

+ +
+
Attached file
+
📎 {{ filename }}
+
+ + {% if has_metadata and metadata %} +

Document metadata

+ + {% for key, value in metadata.items() %} + + + + + {% endfor %} +
{{ key }}{{ value }}
+ {% endif %} + +

+ This document was sent automatically by {{ app_name }}.{% if app_url %} Visit {{ app_url }} to manage your documents.{% endif %} +

+
+
+ © {{ current_year }} {{ app_name }} · Intelligent Document Processing + {% if app_url %}· {{ app_url }}{% endif %} +
+
+ + diff --git a/app/utils/config_validator/providers.py b/app/utils/config_validator/providers.py index 5880ee91..db278d0c 100644 --- a/app/utils/config_validator/providers.py +++ b/app/utils/config_validator/providers.py @@ -154,23 +154,23 @@ def get_provider_status() -> dict[str, dict[str, object]]: }, } - # Add Email configuration + # Add Email destination configuration (dedicated settings for document delivery) providers["Email"] = { "name": "Email", "icon": "fa-solid fa-envelope", "configured": bool( - getattr(settings, "email_host", None) and getattr(settings, "email_default_recipient", None) + getattr(settings, "dest_email_host", None) and getattr(settings, "dest_email_default_recipient", None) ), "enabled": True, "description": "Send documents via email", "details": { - "host": getattr(settings, "email_host", "Not set"), - "port": getattr(settings, "email_port", "Not set"), - "username": getattr(settings, "email_username", "Not set"), - "password": mask_sensitive_value(getattr(settings, "email_password", None)), - "use_tls": getattr(settings, "email_use_tls", "Not set"), - "sender": getattr(settings, "email_sender", "Not set"), - "default_recipient": getattr(settings, "email_default_recipient", "Not set"), + "host": getattr(settings, "dest_email_host", "Not set"), + "port": getattr(settings, "dest_email_port", "Not set"), + "username": getattr(settings, "dest_email_username", "Not set"), + "password": mask_sensitive_value(getattr(settings, "dest_email_password", None)), + "use_tls": getattr(settings, "dest_email_use_tls", "Not set"), + "sender": getattr(settings, "dest_email_sender", "Not set"), + "default_recipient": getattr(settings, "dest_email_default_recipient", "Not set"), }, } diff --git a/app/utils/config_validator/validators.py b/app/utils/config_validator/validators.py index 394e6952..36fa02e6 100644 --- a/app/utils/config_validator/validators.py +++ b/app/utils/config_validator/validators.py @@ -107,12 +107,12 @@ def validate_storage_configs() -> dict[str, list[str]]: issues["sftp"] = sftp_issues - # Validate Email sending + # Validate Email sending (destination-specific settings) email_issues = [] - if not getattr(settings, "email_host", None): - email_issues.append("EMAIL_HOST is not configured") - if not getattr(settings, "email_default_recipient", None): - email_issues.append("EMAIL_DEFAULT_RECIPIENT is not configured") + if not getattr(settings, "dest_email_host", None): + email_issues.append("DEST_EMAIL_HOST is not configured") + if not getattr(settings, "dest_email_default_recipient", None): + email_issues.append("DEST_EMAIL_DEFAULT_RECIPIENT is not configured") issues["email"] = email_issues # Validate S3 diff --git a/app/utils/settings_service.py b/app/utils/settings_service.py index ed2738e5..80dfa0b3 100644 --- a/app/utils/settings_service.py +++ b/app/utils/settings_service.py @@ -917,7 +917,7 @@ SETTING_METADATA = { # Email Settings "email_host": { "category": "Email", - "description": "SMTP server hostname", + "description": "SMTP server hostname (shared – used for password reset and verification emails)", "type": "string", "sensitive": False, "required": False, @@ -965,7 +965,64 @@ SETTING_METADATA = { }, "email_default_recipient": { "category": "Email", - "description": "Default recipient email address", + "description": "Default recipient email address (shared – used for system notifications)", + "type": "string", + "sensitive": False, + "required": False, + "restart_required": False, + }, + # Email Destination Settings (dedicated SMTP for document delivery) + "dest_email_host": { + "category": "Email Destination", + "description": "SMTP server hostname for document delivery (separate from shared email settings)", + "type": "string", + "sensitive": False, + "required": False, + "restart_required": False, + }, + "dest_email_port": { + "category": "Email Destination", + "description": "SMTP port for document delivery (default: 587)", + "type": "integer", + "sensitive": False, + "required": False, + "restart_required": False, + }, + "dest_email_username": { + "category": "Email Destination", + "description": "SMTP username for document delivery", + "type": "string", + "sensitive": False, + "required": False, + "restart_required": False, + }, + "dest_email_password": { + "category": "Email Destination", + "description": "SMTP password for document delivery", + "type": "string", + "sensitive": True, + "required": False, + "restart_required": False, + }, + "dest_email_use_tls": { + "category": "Email Destination", + "description": "Use TLS encryption for document delivery SMTP", + "type": "boolean", + "sensitive": False, + "required": False, + "restart_required": False, + }, + "dest_email_sender": { + "category": "Email Destination", + "description": "From address for document delivery emails (defaults to dest_email_username)", + "type": "string", + "sensitive": False, + "required": False, + "restart_required": False, + }, + "dest_email_default_recipient": { + "category": "Email Destination", + "description": "Default recipient email for document delivery when none is specified", "type": "string", "sensitive": False, "required": False, diff --git a/docs/ConfigurationGuide.md b/docs/ConfigurationGuide.md index 2de94abd..a48c4d97 100644 --- a/docs/ConfigurationGuide.md +++ b/docs/ConfigurationGuide.md @@ -954,7 +954,11 @@ For detailed setup instructions, see the [Google Drive Setup Guide](GoogleDriveS | `SFTP_PRIVATE_KEY` | Path to private key file for authentication (optional). | | `SFTP_PRIVATE_KEY_PASSPHRASE`| Passphrase for private key if required (optional). | -### Email +### Email (shared SMTP – password reset & verification) + +> **Note:** These settings configure the shared SMTP connection used for system emails such as +> password resets and account verification. They do **not** enable the email delivery destination. +> To send processed documents via email, configure the dedicated `DEST_EMAIL_*` variables below. | **Variable** | **Description** | |----------------------------|----------------------------------------------------------| @@ -964,7 +968,22 @@ For detailed setup instructions, see the [Google Drive Setup Guide](GoogleDriveS | `EMAIL_PASSWORD` | SMTP authentication password. | | `EMAIL_USE_TLS` | Whether to use TLS (default: `True`). | | `EMAIL_SENDER` | From address (e.g., `"DocuElevate "`). | -| `EMAIL_DEFAULT_RECIPIENT` | Default recipient email if none specified in the task. | + +### Email Destination (document delivery) + +> **Note:** These settings are intentionally separate from the shared `EMAIL_*` settings above. +> Configuring `EMAIL_HOST` for password resets does **not** automatically activate the email +> delivery destination. You must set `DEST_EMAIL_HOST` to enable it. + +| **Variable** | **Description** | +|----------------------------------|---------------------------------------------------------------------| +| `DEST_EMAIL_HOST` | SMTP server hostname for document delivery. | +| `DEST_EMAIL_PORT` | SMTP port for document delivery (default: `587`). | +| `DEST_EMAIL_USERNAME` | SMTP authentication username for document delivery. | +| `DEST_EMAIL_PASSWORD` | SMTP authentication password for document delivery. | +| `DEST_EMAIL_USE_TLS` | Whether to use TLS for document delivery (default: `True`). | +| `DEST_EMAIL_SENDER` | From address for delivered documents (e.g., `"DocuElevate Delivery "`). | +| `DEST_EMAIL_DEFAULT_RECIPIENT` | Fallback recipient email when none is specified for a delivery task. | ### OneDrive / Microsoft Graph @@ -1362,14 +1381,22 @@ SFTP_FOLDER=/Documents/Uploads # SFTP_PRIVATE_KEY=/path/to/key.pem # SFTP_PRIVATE_KEY_PASSPHRASE=passphrase -# Email +# Email (shared SMTP – password reset & verification) EMAIL_HOST=smtp.example.com EMAIL_PORT=587 EMAIL_USERNAME=docuelevate@example.com EMAIL_PASSWORD=password EMAIL_USE_TLS=True EMAIL_SENDER=DocuElevate System -EMAIL_DEFAULT_RECIPIENT=recipient@example.com + +# Email Destination (document delivery – separate from shared email above) +DEST_EMAIL_HOST=smtp.example.com +DEST_EMAIL_PORT=587 +DEST_EMAIL_USERNAME=docuelevate@example.com +DEST_EMAIL_PASSWORD=password +DEST_EMAIL_USE_TLS=True +DEST_EMAIL_SENDER=DocuElevate Delivery +DEST_EMAIL_DEFAULT_RECIPIENT=recipient@example.com # Notification Settings # Configure notification services using Apprise URL format diff --git a/tests/test_auth.py b/tests/test_auth.py index ccb2dd7c..052c1f6e 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -174,6 +174,61 @@ class TestRequireLogin: assert result["message"] == "sync" assert result["param"] == "test_value" + @pytest.mark.asyncio + async def test_returns_401_for_api_paths_when_not_authenticated(self): + """Test that require_login returns 401 (not redirect) for /api/* paths. + + This prevents the /api/auth/whoami JS probe from overwriting + redirect_after_login with an API URL, which would send the user to a + JSON endpoint after login instead of the page they actually wanted. + """ + from fastapi.responses import JSONResponse + + with patch("app.auth.AUTH_ENABLED", True): + from app.auth import require_login + + @require_login + async def api_endpoint(request: Request): + return {"message": "success"} + + mock_request = MagicMock(spec=Request) + mock_request.session = {} + mock_request.url = MagicMock() + mock_request.url.__str__ = MagicMock(return_value="http://test.com/api/auth/whoami") + + result = await api_endpoint(mock_request) + + assert isinstance(result, JSONResponse) + assert result.status_code == status.HTTP_401_UNAUTHORIZED + # Redirect URL must NOT be stored for API paths + assert "redirect_after_login" not in mock_request.session + + @pytest.mark.asyncio + async def test_does_not_save_redirect_for_api_paths(self): + """Test that redirect_after_login is never set for any /api/* request.""" + from fastapi.responses import JSONResponse + + with patch("app.auth.AUTH_ENABLED", True): + from app.auth import require_login + + @require_login + async def api_endpoint(request: Request): + return {"data": "ok"} + + for api_path in ["/api/documents/upload", "/api/v1/resource", "/api/users/me"]: + mock_request = MagicMock(spec=Request) + mock_request.session = {} + mock_request.url = MagicMock() + mock_request.url.__str__ = MagicMock(return_value=f"http://test.com{api_path}") + + result = await api_endpoint(mock_request) + + assert isinstance(result, JSONResponse), f"Expected JSONResponse for {api_path}" + assert result.status_code == status.HTTP_401_UNAUTHORIZED + assert "redirect_after_login" not in mock_request.session, ( + f"redirect_after_login must not be set for {api_path}" + ) + @pytest.mark.asyncio async def test_path_param_before_request_async(self): """Regression: endpoints with a path param before request must not get diff --git a/tests/test_auth_module.py b/tests/test_auth_module.py index d08830ff..233dfd5e 100644 --- a/tests/test_auth_module.py +++ b/tests/test_auth_module.py @@ -185,6 +185,33 @@ class TestRequireLogin: assert isinstance(result, RedirectResponse) assert result.status_code == status.HTTP_302_FOUND + @pytest.mark.asyncio + async def test_returns_401_for_api_path_when_not_authenticated(self): + """Test returns 401 for /api/* paths instead of redirect-to-login. + + Prevents the common.js /api/auth/whoami probe from overwriting + redirect_after_login, which would send the user to a JSON endpoint + after login instead of the page they originally requested. + """ + from fastapi.responses import JSONResponse + + with patch("app.auth.AUTH_ENABLED", True): + + @require_login + async def test_api_endpoint(request: Request): + return {"data": "ok"} + + mock_request = MagicMock(spec=Request) + mock_request.session = {} + mock_request.url = MagicMock() + mock_request.url.__str__ = MagicMock(return_value="http://localhost/api/auth/whoami") + + result = await test_api_endpoint(mock_request) + + assert isinstance(result, JSONResponse) + assert result.status_code == status.HTTP_401_UNAUTHORIZED + assert "redirect_after_login" not in mock_request.session + @pytest.mark.unit class TestOAuthConfiguration: diff --git a/tests/test_config_validators.py b/tests/test_config_validators.py index 8880b58e..308f2e0b 100644 --- a/tests/test_config_validators.py +++ b/tests/test_config_validators.py @@ -75,13 +75,13 @@ class TestValidateStorageConfigs: assert "Neither SFTP_KEY_PATH nor SFTP_PASSWORD is configured" in result["sftp"] def test_email_storage_missing_config(self): - """Test validation when email storage config is missing.""" + """Test validation when email destination storage config is missing.""" with patch("app.utils.config_validator.validators.settings") as mock_settings: - mock_settings.email_host = None - mock_settings.email_default_recipient = None + mock_settings.dest_email_host = None + mock_settings.dest_email_default_recipient = None result = validate_storage_configs() - assert "EMAIL_HOST is not configured" in result["email"] - assert "EMAIL_DEFAULT_RECIPIENT is not configured" in result["email"] + assert "DEST_EMAIL_HOST is not configured" in result["email"] + assert "DEST_EMAIL_DEFAULT_RECIPIENT is not configured" in result["email"] @pytest.mark.unit @@ -438,8 +438,8 @@ class TestValidateStorageConfigsEdgeCases: # Configure all services mock_settings.sftp_host = "sftp.example.com" mock_settings.sftp_password = "pass" - mock_settings.email_host = "smtp.example.com" - mock_settings.email_default_recipient = "test@example.com" + mock_settings.dest_email_host = "smtp.example.com" + mock_settings.dest_email_default_recipient = "test@example.com" mock_settings.s3_bucket_name = "my-bucket" mock_settings.aws_access_key_id = "key" mock_settings.aws_secret_access_key = "secret" diff --git a/tests/test_coverage_remaining_gaps.py b/tests/test_coverage_remaining_gaps.py index 642a1f86..5524ea71 100644 --- a/tests/test_coverage_remaining_gaps.py +++ b/tests/test_coverage_remaining_gaps.py @@ -620,6 +620,13 @@ def _set_minimal_provider_settings(mock_settings): "email_password": None, "email_use_tls": True, "email_sender": None, + "dest_email_host": None, + "dest_email_default_recipient": None, + "dest_email_port": 587, + "dest_email_username": None, + "dest_email_password": None, + "dest_email_use_tls": True, + "dest_email_sender": None, "ftp_host": None, "ftp_username": None, "ftp_password": None, diff --git a/tests/test_finalize_storage.py b/tests/test_finalize_storage.py index cb268b22..a016f242 100644 --- a/tests/test_finalize_storage.py +++ b/tests/test_finalize_storage.py @@ -372,3 +372,69 @@ class TestFinalizeDocumentStorage: # Verify send_to_all was called with delete_after=True mock_send_all.delay.assert_called_once_with("/workdir/processed/file.pdf", True, 505) + + @patch("app.tasks.finalize_document_storage.notify_file_processed") + @patch("app.tasks.finalize_document_storage.send_to_all_destinations") + @patch("app.tasks.finalize_document_storage.get_configured_services_from_validator") + @patch("app.tasks.finalize_document_storage.log_task_progress") + @patch("app.tasks.finalize_document_storage.SessionLocal") + def test_pdfa_enabled_does_not_regress_finalize_step_to_in_progress( + self, + mock_session_local, + mock_log_progress, + mock_get_services, + mock_send_all, + mock_notify, + ): + """ + Regression test: when PDF/A conversion is enabled, the finalize_document_storage + step must NOT be logged as in_progress after it has already been logged as success. + + Previously, a second log_task_progress call with status="in_progress" was made for + "finalize_document_storage" when queueing PDF/A archival conversion, which overwrote + the prior success status and caused the overall file status to appear stuck in + processing/failed. + """ + mock_get_services.return_value = {"dropbox": True} + + mock_db = MagicMock() + mock_session_local.return_value.__enter__.return_value = mock_db + mock_db.query.return_value.filter.return_value.first.return_value = None + + with patch("app.tasks.finalize_document_storage.os.path.exists", return_value=True): + with patch("app.tasks.finalize_document_storage.os.path.getsize", return_value=1024): + with patch("app.tasks.finalize_document_storage.os.path.basename", return_value="doc.pdf"): + with patch("app.tasks.finalize_document_storage.settings") as mock_settings: + mock_settings.workdir = "/tmp" + mock_settings.enable_pdfa_conversion = True + + mock_convert = MagicMock() + with patch( + "app.tasks.finalize_document_storage.convert_to_pdfa", + mock_convert, + create=True, + ): + finalize_document_storage.request.id = "test-task-id" + + finalize_document_storage.__wrapped__( + original_file="/tmp/original.pdf", + processed_file="/workdir/processed/doc.pdf", + metadata={"filename": "doc.pdf"}, + file_id=606, + ) + + # Collect all (step_name, status) pairs logged for finalize_document_storage + finalize_calls = [ + call + for call in mock_log_progress.call_args_list + if call.args[1] == "finalize_document_storage" + ] + + # After the success log, no in_progress log should follow for this step + statuses = [call.args[2] for call in finalize_calls] + assert "success" in statuses, "finalize_document_storage must be logged as success" + # The last status logged must be success, not in_progress + assert statuses[-1] == "success", ( + "finalize_document_storage must not be regressed to in_progress after success; " + f"got statuses: {statuses}" + ) diff --git a/tests/test_send_to_all.py b/tests/test_send_to_all.py index 2f549844..d4632a6d 100644 --- a/tests/test_send_to_all.py +++ b/tests/test_send_to_all.py @@ -120,10 +120,10 @@ class TestShouldUploadFunctions: @patch("app.tasks.send_to_all.settings") def test_should_upload_to_email_configured(self, mock_settings): """Test email upload check.""" - mock_settings.email_host = "smtp.example.com" - mock_settings.email_username = "user" - mock_settings.email_password = "pass" - mock_settings.email_default_recipient = "recipient@example.com" + mock_settings.dest_email_host = "smtp.example.com" + mock_settings.dest_email_username = "user" + mock_settings.dest_email_password = "pass" + mock_settings.dest_email_default_recipient = "recipient@example.com" assert _should_upload_to_email() is True diff --git a/tests/test_upload_email.py b/tests/test_upload_email.py index 3e058881..c92893a6 100644 --- a/tests/test_upload_email.py +++ b/tests/test_upload_email.py @@ -229,7 +229,7 @@ class TestPrepareRecipients: @patch("app.tasks.upload_to_email.settings") def test_uses_default_recipient_when_none_provided(self, mock_settings): """Test uses default recipient when none provided.""" - mock_settings.email_default_recipient = "default@example.com" + mock_settings.dest_email_default_recipient = "default@example.com" result, error = _prepare_recipients(None) @@ -239,7 +239,7 @@ class TestPrepareRecipients: @patch("app.tasks.upload_to_email.settings") def test_returns_error_when_no_recipients_and_no_default(self, mock_settings): """Test returns error when no recipients and no default.""" - mock_settings.email_default_recipient = None + mock_settings.dest_email_default_recipient = None result, error = _prepare_recipients(None) @@ -256,11 +256,11 @@ class TestSendEmailWithSMTP: @patch("app.tasks.upload_to_email.settings") def test_sends_email_successfully(self, mock_settings, mock_gethostbyname, mock_smtp): """Test sends email successfully.""" - mock_settings.email_host = "smtp.example.com" - mock_settings.email_port = 587 - mock_settings.email_use_tls = True - mock_settings.email_username = "user@example.com" - mock_settings.email_password = "password" + mock_settings.dest_email_host = "smtp.example.com" + mock_settings.dest_email_port = 587 + mock_settings.dest_email_use_tls = True + mock_settings.dest_email_username = "user@example.com" + mock_settings.dest_email_password = "password" mock_server = MagicMock() mock_smtp.return_value.__enter__.return_value = mock_server @@ -292,8 +292,8 @@ class TestSendEmailWithSMTP: @patch("app.tasks.upload_to_email.settings") def test_handles_connection_refused_error(self, mock_settings, mock_gethostbyname, mock_smtp): """Test handles connection refused error.""" - mock_settings.email_host = "smtp.example.com" - mock_settings.email_port = 587 + mock_settings.dest_email_host = "smtp.example.com" + mock_settings.dest_email_port = 587 mock_smtp.return_value.__enter__.side_effect = ConnectionRefusedError("Connection refused") @@ -309,11 +309,11 @@ class TestSendEmailWithSMTP: @patch("app.tasks.upload_to_email.settings") def test_sends_email_without_tls(self, mock_settings, mock_gethostbyname, mock_smtp): """Test sends email without TLS.""" - mock_settings.email_host = "smtp.example.com" - mock_settings.email_port = 25 - mock_settings.email_use_tls = False - mock_settings.email_username = "user@example.com" - mock_settings.email_password = "password" + mock_settings.dest_email_host = "smtp.example.com" + mock_settings.dest_email_port = 25 + mock_settings.dest_email_use_tls = False + mock_settings.dest_email_username = "user@example.com" + mock_settings.dest_email_password = "password" mock_server = MagicMock() mock_smtp.return_value.__enter__.return_value = mock_server @@ -333,11 +333,11 @@ class TestSendEmailWithSMTP: @patch("app.tasks.upload_to_email.settings") def test_sends_email_without_authentication(self, mock_settings, mock_gethostbyname, mock_smtp): """Test sends email without authentication credentials.""" - mock_settings.email_host = "smtp.example.com" - mock_settings.email_port = 25 - mock_settings.email_use_tls = False - mock_settings.email_username = None - mock_settings.email_password = None + mock_settings.dest_email_host = "smtp.example.com" + mock_settings.dest_email_port = 25 + mock_settings.dest_email_use_tls = False + mock_settings.dest_email_username = None + mock_settings.dest_email_password = None mock_server = MagicMock() mock_smtp.return_value.__enter__.return_value = mock_server @@ -356,8 +356,8 @@ class TestSendEmailWithSMTP: @patch("app.tasks.upload_to_email.settings") def test_handles_timeout_error(self, mock_settings, mock_gethostbyname, mock_smtp): """Test handles timeout error.""" - mock_settings.email_host = "smtp.example.com" - mock_settings.email_port = 587 + mock_settings.dest_email_host = "smtp.example.com" + mock_settings.dest_email_port = 587 mock_smtp.return_value.__enter__.side_effect = TimeoutError("Connection timeout") @@ -392,10 +392,10 @@ class TestUploadToEmailTask: @patch("app.tasks.upload_to_email.os.path.exists") @patch("app.tasks.upload_to_email.settings") def test_skips_when_email_host_not_configured(self, mock_settings, mock_exists, mock_log, mock_basename): - """Test skips when email host not configured.""" + """Test skips when email destination host not configured.""" mock_exists.return_value = True mock_basename.return_value = "test.pdf" - mock_settings.email_host = None + mock_settings.dest_email_host = None mock_self = Mock() mock_self.request.id = "test-task-id" @@ -403,7 +403,7 @@ class TestUploadToEmailTask: result = upload_to_email(mock_self, "/tmp/test.pdf") assert result["status"] == "Skipped" - assert "Email host is not configured" in result["reason"] + assert "DEST_EMAIL_HOST" in result["reason"] @patch("app.tasks.upload_to_email.os.path.basename") @patch("app.tasks.upload_to_email._prepare_recipients") @@ -414,7 +414,7 @@ class TestUploadToEmailTask: """Test skips when no valid recipients.""" mock_exists.return_value = True mock_basename.return_value = "test.pdf" - mock_settings.email_host = "smtp.example.com" + mock_settings.dest_email_host = "smtp.example.com" mock_prepare.return_value = (None, "No recipients specified") mock_self = Mock() diff --git a/tests/test_upload_tasks.py b/tests/test_upload_tasks.py index 4c4c63e3..03a40601 100644 --- a/tests/test_upload_tasks.py +++ b/tests/test_upload_tasks.py @@ -364,12 +364,12 @@ def test_upload_to_email_accepts_file_id(sample_text_file): patch("app.tasks.upload_to_email.attach_logo") as mock_logo, ): # Setup settings - mock_settings.email_host = "smtp.example.com" - mock_settings.email_port = 587 - mock_settings.email_username = "test@example.com" - mock_settings.email_password = _TEST_CREDENTIAL - mock_settings.email_use_tls = True - mock_settings.email_sender = "sender@example.com" + mock_settings.dest_email_host = "smtp.example.com" + mock_settings.dest_email_port = 587 + mock_settings.dest_email_username = "test@example.com" + mock_settings.dest_email_password = _TEST_CREDENTIAL + mock_settings.dest_email_use_tls = True + mock_settings.dest_email_sender = "sender@example.com" mock_settings.external_hostname = "docuelevate.example.com" # Setup mocks @@ -503,7 +503,7 @@ def test_send_to_all_calls_upload_tasks_with_keyword_argument(): mock_settings.webdav_url = None mock_settings.ftp_host = None mock_settings.sftp_host = None - mock_settings.email_host = None + mock_settings.dest_email_host = None mock_settings.onedrive_client_id = None mock_settings.workdir = "/tmp"