From aa7d5d7c7ca39dcb981dcc516cbe460a993fe409 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Mar 2026 09:39:11 +0000 Subject: [PATCH 01/25] Initial plan From 8d65f0940077e05bb8c53232d726fd76b0717f2b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Mar 2026 09:41:06 +0000 Subject: [PATCH 02/25] Initial plan From 33a02633b706031083d22b445903007c2a64b659 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Mar 2026 09:42:28 +0000 Subject: [PATCH 03/25] Initial plan From 44ea43f9cf948aabe8f8ae98744cd41061ab8770 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Mar 2026 09:44:59 +0000 Subject: [PATCH 04/25] chore: update plan to include forgot-username and login label clarification Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> --- app/api/admin_users.py | 154 +++++++++++++- app/api/local_auth.py | 13 ++ frontend/templates/admin_users.html | 255 +++++++++++++++++++++++- frontend/templates/forgot_password.html | 118 +++++++++++ frontend/templates/login.html | 5 + 5 files changed, 540 insertions(+), 5 deletions(-) create mode 100644 frontend/templates/forgot_password.html diff --git a/app/api/admin_users.py b/app/api/admin_users.py index c0f92cbd..1e5fc221 100644 --- a/app/api/admin_users.py +++ b/app/api/admin_users.py @@ -7,7 +7,7 @@ user accounts directly, without requiring email verification. """ import logging -from datetime import datetime +from datetime import datetime, timezone from typing import Annotated, Any from fastapi import APIRouter, Depends, HTTPException, Query, Request, status @@ -17,7 +17,7 @@ from sqlalchemy.orm import Session from app.database import get_db from app.models import FileRecord, LocalUser, UserProfile -from app.utils.local_auth import hash_password +from app.utils.local_auth import generate_token, hash_password, send_password_reset_email logger = logging.getLogger(__name__) router = APIRouter(prefix="/admin/users", tags=["admin-users"]) @@ -123,6 +123,21 @@ class LocalUserCreate(BaseModel): is_admin: bool = Field(default=False, description="Grant admin privileges") +class LocalUserUpdate(BaseModel): + """Body for admin-updating a local (email/password) user account.""" + + email: str | None = Field(default=None, max_length=255, description="New email address") + display_name: str | None = Field(default=None, max_length=255, description="New display name") + is_admin: bool | None = Field(default=None, description="Grant or revoke admin privileges") + is_active: bool | None = Field(default=None, description="Activate or deactivate the account") + + +class LocalUserSetPassword(BaseModel): + """Body for admin setting a temporary password for a local user.""" + + password: str = Field(..., min_length=8, max_length=128, description="New temporary password") + + class LocalUserResponse(BaseModel): """Summary of a local user account.""" @@ -355,7 +370,140 @@ def delete_local_user(local_user_id: int, db: DbSession, _admin: AdminUser) -> N logger.info("Admin deleted local user account: %s", user.email) -@router.get("/{user_id:path}", summary="Get details for a single user") +@router.patch("/local/{local_user_id}", summary="Update a local user account") +def update_local_user(local_user_id: int, body: LocalUserUpdate, db: DbSession, _admin: AdminUser) -> dict[str, Any]: + """Update the email address, display name, admin flag, or active status of a local user account. + + Only fields explicitly provided (non-None) are modified. If the email is changed + the associated UserProfile row is also updated to keep ``user_id`` in sync. + + Raises: + 404: Local user not found. + 409: The new email is already taken by another account. + """ + user = db.query(LocalUser).filter(LocalUser.id == local_user_id).first() + if not user: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Local user not found.") + + old_email = user.email + + if body.email is not None and body.email != user.email: + if db.query(LocalUser).filter(LocalUser.email == body.email, LocalUser.id != local_user_id).first(): + raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail="Email already registered.") + user.email = body.email + + if body.display_name is not None: + user.display_name = body.display_name + + if body.is_admin is not None: + user.is_admin = body.is_admin + + if body.is_active is not None: + user.is_active = body.is_active + + try: + db.flush() + # Keep UserProfile.user_id in sync when email changes + if body.email is not None and body.email != old_email: + profile = db.query(UserProfile).filter(UserProfile.user_id == old_email).first() + if profile: + profile.user_id = body.email + db.commit() + db.refresh(user) + except Exception: + db.rollback() + raise + + logger.info("Admin updated local user %s (id=%d)", user.email, user.id) + return { + "id": user.id, + "email": user.email, + "username": user.username, + "display_name": user.display_name, + "is_active": user.is_active, + "is_admin": user.is_admin, + "created_at": user.created_at.isoformat() if user.created_at else None, + } + + +@router.post( + "/local/{local_user_id}/send-password-reset", + status_code=status.HTTP_200_OK, + summary="Send a password reset email to a local user", +) +def admin_send_password_reset( + local_user_id: int, request: Request, db: DbSession, _admin: AdminUser +) -> dict[str, Any]: + """Generate a password reset token and email the reset link to the local user. + + This is a last-resort tool for admins to help users who are locked out. + Returns ``{"sent": true}`` on success and ``{"sent": false, "reason": "..."}`` when + SMTP is not configured or sending fails. + + Raises: + 404: Local user not found. + """ + from app.config import settings as _settings + + user = db.query(LocalUser).filter(LocalUser.id == local_user_id).first() + if not user: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Local user not found.") + + if not _settings.email_host: + logger.warning("Admin requested password reset for %s but SMTP is not configured", user.email) + return {"sent": False, "reason": "SMTP is not configured on this server."} + + token = generate_token() + user.password_reset_token = token + user.password_reset_sent_at = datetime.now(tz=timezone.utc) + db.commit() + + base_url = str(request.base_url).rstrip("/") + try: + send_password_reset_email(user.email, user.username, token, base_url) + except Exception as exc: + logger.warning("Admin-triggered password reset email failed for %s: %s", user.email, exc) + return {"sent": False, "reason": str(exc)} + + logger.info("[SECURITY] ADMIN_PASSWORD_RESET_EMAIL user=%s admin=%s", user.email, _admin.get("email", "unknown")) + return {"sent": True, "email": user.email} + + +@router.post( + "/local/{local_user_id}/set-password", + status_code=status.HTTP_200_OK, + summary="Set a temporary password for a local user account", +) +def admin_set_password( + local_user_id: int, body: LocalUserSetPassword, db: DbSession, _admin: AdminUser +) -> dict[str, Any]: + """Directly set a new password for a local user without requiring an email token. + + Use this as a last resort when email delivery is unavailable. The user + should be advised to change their password after logging in. + + Raises: + 404: Local user not found. + """ + user = db.query(LocalUser).filter(LocalUser.id == local_user_id).first() + if not user: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Local user not found.") + + user.hashed_password = hash_password(body.password) + # Clear any outstanding reset tokens + user.password_reset_token = None + user.password_reset_sent_at = None + + try: + db.commit() + except Exception: + db.rollback() + raise + + logger.info( + "[SECURITY] ADMIN_SET_PASSWORD user=%s admin=%s", user.email, _admin.get("email", "unknown") + ) + return {"updated": True, "email": user.email} def get_user(user_id: str, db: DbSession, _admin: AdminUser) -> dict[str, Any]: """Return profile and document statistics for a specific user.""" doc_count = db.query(func.count(FileRecord.id)).filter(FileRecord.owner_id == user_id).scalar() or 0 diff --git a/app/api/local_auth.py b/app/api/local_auth.py index 75f341bd..f42cbe2b 100644 --- a/app/api/local_auth.py +++ b/app/api/local_auth.py @@ -107,6 +107,19 @@ async def verify_email_sent_page(request: Request) -> Any: return templates.TemplateResponse("verify_email_sent.html", {"request": request}) +@router.get("/forgot-password", include_in_schema=False) +async def forgot_password_page(request: Request) -> Any: + """Render the forgot-password page where users can request a reset email.""" + return templates.TemplateResponse( + "forgot_password.html", + { + "request": request, + "csrf_token": getattr(request.state, "csrf_token", ""), + "app_version": settings.version, + }, + ) + + @router.get("/reset-password", include_in_schema=False) async def reset_password_page(request: Request) -> Any: """Render the password reset form page.""" diff --git a/frontend/templates/admin_users.html b/frontend/templates/admin_users.html index be8e4fdc..874c1789 100644 --- a/frontend/templates/admin_users.html +++ b/frontend/templates/admin_users.html @@ -508,13 +508,38 @@ + + + @@ -595,6 +620,116 @@ + + + + + +
({})); + this.editLocalUserModal.error = err.detail || 'Failed to update account.'; + } + } catch (e) { + this.editLocalUserModal.error = 'Network error: ' + e.message; + } finally { + this.editLocalUserModal.saving = false; + } + }, + + openSetPasswordModal(lu) { + this.setPasswordModal.id = lu.id; + this.setPasswordModal.username = lu.username; + this.setPasswordModal.password = ''; + this.setPasswordModal.error = ''; + this.setPasswordModal.saving = false; + this.setPasswordModal.open = true; + }, + + async submitSetPassword() { + this.setPasswordModal.error = ''; + this.setPasswordModal.saving = true; + try { + const resp = await fetch(`/api/admin/users/local/${this.setPasswordModal.id}/set-password`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'X-CSRF-Token': document.querySelector('meta[name="csrf-token"]')?.content || '', + }, + body: JSON.stringify({ password: this.setPasswordModal.password }), + }); + if (resp.ok) { + this.setPasswordModal.open = false; + this.showAlert('success', 'Password set', `Password for "${this.setPasswordModal.username}" has been updated.`); + } else { + const err = await resp.json().catch(() => ({})); + this.setPasswordModal.error = err.detail || 'Failed to set password.'; + } + } catch (e) { + this.setPasswordModal.error = 'Network error: ' + e.message; + } finally { + this.setPasswordModal.saving = false; + } + }, + + async sendPasswordReset(lu) { + try { + const resp = await fetch(`/api/admin/users/local/${lu.id}/send-password-reset`, { + method: 'POST', + headers: { + 'X-CSRF-Token': document.querySelector('meta[name="csrf-token"]')?.content || '', + }, + }); + const data = await resp.json().catch(() => ({})); + if (resp.ok && data.sent) { + this.showAlert('success', 'Email sent', `Password reset email sent to "${lu.email}".`); + } else if (resp.ok && !data.sent) { + this.showAlert('error', 'Email not sent', data.reason || 'SMTP is not configured.'); + } else { + this.showAlert('error', 'Failed', data.detail || resp.statusText); + } + } catch (e) { + this.showAlert('error', 'Network error', e.message); + } + }, + async executeDeleteLocalUser() { this.deleteLocalUserModal.deleting = true; try { diff --git a/frontend/templates/forgot_password.html b/frontend/templates/forgot_password.html new file mode 100644 index 00000000..148f01fe --- /dev/null +++ b/frontend/templates/forgot_password.html @@ -0,0 +1,118 @@ + + + + + + DocuElevate - Forgot Password + + + + + +
+
+ DocuElevate Logo +
+ +

Forgot your password?

+

+ Enter your email address and we'll send you a link to reset your password. +

+ +
+
+
+
+ +
+
+

Check your inbox

+

+ If an account exists for that email address, a password reset link has been sent. The link expires in 24 hours. +

+ Back to sign in +
+ +
+ + +
+ + +
+ + +
+
+ + +
+
+ DocuElevate {{ app_version|default('', true) }} +
+ + diff --git a/frontend/templates/login.html b/frontend/templates/login.html index 5ead49b9..0165035a 100644 --- a/frontend/templates/login.html +++ b/frontend/templates/login.html @@ -50,6 +50,11 @@ Sign in +
+ + Forgot your password? + +
{% if show_oauth %} From 4857203d08966e5c470517255f8e2d74854cbbc1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Mar 2026 09:49:29 +0000 Subject: [PATCH 05/25] Initial plan From 3aa5364e0ca3eaceb37616bb9b3a9a55fc08b223 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Mar 2026 09:51:23 +0000 Subject: [PATCH 06/25] fix(auth): return 401 for API paths in require_login to prevent wrong post-login redirect The common.js fetch('/api/auth/whoami') probe on every page load was overwriting the redirect_after_login session key with the API endpoint URL. After login, users were sent to the JSON endpoint instead of the original page. Fix: require_login now returns HTTP 401 for any /api/* path, consistent with REST conventions, and never stores API URLs as the post-login redirect. Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> --- app/auth.py | 13 +++++++++ tests/test_auth.py | 55 +++++++++++++++++++++++++++++++++++++++ tests/test_auth_module.py | 27 +++++++++++++++++++ 3 files changed, 95 insertions(+) diff --git a/app/auth.py b/app/auth.py index d6b2ec39..f570bc2a 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) # Check if the wrapped function is a coroutine function diff --git a/tests/test_auth.py b/tests/test_auth.py index c16fe318..81b6276e 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -175,6 +175,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.integration class TestWhoamiEndpoint: 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: From 9b45ba62ba1e9f491765bd41304df191f2042104 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Mar 2026 09:53:39 +0000 Subject: [PATCH 07/25] Initial plan From a0f5ba179978d7c63bbdb8c564a32eaf3f78e5ad Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Mar 2026 09:56:58 +0000 Subject: [PATCH 08/25] feat(backup): extend backup and restore to PostgreSQL and MySQL/MariaDB Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> --- app/api/backup.py | 152 ++++++----- app/tasks/backup_tasks.py | 346 ++++++++++++++++++++++++- docs/ConfigurationGuide.md | 6 +- docs/DatabaseConfiguration.md | 80 +++++- tests/test_backup.py | 461 ++++++++++++++++++++++++++++++++-- 5 files changed, 941 insertions(+), 104 deletions(-) diff --git a/app/api/backup.py b/app/api/backup.py index 2c83058c..8262814c 100644 --- a/app/api/backup.py +++ b/app/api/backup.py @@ -117,88 +117,104 @@ async def restore_backup( """Restore the database from an uploaded gzip-compressed SQL dump. **Warning**: This overwrites the current database contents. - Only SQLite databases are supported. - The uploaded file must be a ``.db.gz`` file produced by the DocuElevate - backup task (a gzip-compressed SQLite ``.dump()`` SQL script). + Supported formats (must match the currently configured database backend): + + - ``*.db.gz`` – gzip-compressed SQLite ``.dump()`` SQL script (SQLite backend) + - ``*.pgsql.gz`` – gzip-compressed ``pg_dump --format=plain`` output (PostgreSQL backend) + - ``*.mysql.gz`` – gzip-compressed ``mysqldump`` output (MySQL / MariaDB backend) """ - from app.tasks.backup_tasks import _db_path - - db_path = _db_path() - if db_path is None: - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail="Restore is only supported for SQLite databases.", - ) - - if not file.filename or not file.filename.endswith(".db.gz"): - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail="Uploaded file must be a .db.gz backup archive.", - ) - - import gzip - import sqlite3 import tempfile from pathlib import Path - # Write the upload to a temp file first so we can validate it - with tempfile.NamedTemporaryFile(suffix=".db.gz", delete=False) as tmp: + from sqlalchemy.engine.url import make_url + + from app.config import settings as app_settings + from app.tasks.backup_tasks import ( + _archive_ext_for_backend, + _db_path, + _restore_mysql, + _restore_postgresql, + _restore_sqlite, + ) + + url = make_url(app_settings.database_url) + backend = url.get_backend_name() + expected_ext = _archive_ext_for_backend(backend) + + if not file.filename or not file.filename.endswith(expected_ext): + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=( + f"Uploaded file must be a '{expected_ext}' backup archive for the current database backend ({backend})." + ), + ) + + # Write upload to a temp file + with tempfile.NamedTemporaryFile(suffix=expected_ext, delete=False) as tmp: tmp_path = Path(tmp.name) content = await file.read() tmp.write(content) try: - # Decompress and read SQL statements - with gzip.open(str(tmp_path), "rt", encoding="utf-8") as gz: - sql_script = gz.read() - except Exception as exc: - tmp_path.unlink(missing_ok=True) - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail=f"Failed to decompress backup file: {exc}", - ) from exc + if backend == "sqlite": + db_path = _db_path() + if db_path is None: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Restore is only supported for file-based SQLite databases.", + ) + # Close the application DB session before replacing the file + db.close() + try: + _restore_sqlite(db_path, tmp_path) + except ValueError as exc: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=str(exc), + ) from exc + except RuntimeError as exc: + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail=str(exc), + ) from exc - # Create a fresh in-memory DB from the script to validate it - try: - mem_conn = sqlite3.connect(":memory:") - mem_conn.executescript(sql_script) - mem_conn.close() - except sqlite3.Error as exc: - tmp_path.unlink(missing_ok=True) - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail=f"Backup file contains invalid SQL: {exc}", - ) from exc + elif backend == "postgresql": + db.close() + try: + _restore_postgresql(app_settings.database_url, tmp_path) + except FileNotFoundError as exc: + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail=f"psql binary not found – is PostgreSQL client installed? ({exc})", + ) from exc + except RuntimeError as exc: + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail=f"PostgreSQL restore failed: {exc}", + ) from exc - # Close the application DB session before replacing the file - db.close() + elif backend == "mysql": + db.close() + try: + _restore_mysql(app_settings.database_url, tmp_path) + except FileNotFoundError as exc: + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail=f"mysql binary not found – is MySQL client installed? ({exc})", + ) from exc + except RuntimeError as exc: + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail=f"MySQL restore failed: {exc}", + ) from exc - # Preserve the current DB before overwriting - import shutil + else: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=f"Database backend '{backend}' does not support restore.", + ) - bak = str(db_path) + ".pre_restore" - try: - shutil.copy2(str(db_path), bak) - except OSError as exc: - logger.warning(f"Could not create pre-restore backup at {bak}: {exc}") - - try: - # Write the restored database - restore_conn = sqlite3.connect(str(db_path)) - restore_conn.executescript(sql_script) - restore_conn.close() - except sqlite3.Error as exc: - # Attempt rollback - try: - if os.path.exists(bak): - shutil.copy2(bak, str(db_path)) - except OSError as rollback_exc: - logger.error(f"Rollback failed; database may be corrupted: {rollback_exc}") - raise HTTPException( - status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, - detail=f"Restore failed: {exc}", - ) from exc finally: tmp_path.unlink(missing_ok=True) diff --git a/app/tasks/backup_tasks.py b/app/tasks/backup_tasks.py index c94ca1eb..f40014f7 100644 --- a/app/tasks/backup_tasks.py +++ b/app/tasks/backup_tasks.py @@ -16,12 +16,19 @@ Three separate Celery-beat entries call ``create_backup`` with the appropriate After each backup is created ``_apply_retention`` prunes old local backups for that tier. Remote copies are pruned by ``_prune_remote_backups`` which mirrors the same retention limits. + +Supported database backends +---------------------------- +- **SQLite** – dumped via Python's built-in ``sqlite3.iterdump()``; archive extension ``.db.gz`` +- **PostgreSQL** – dumped via ``pg_dump --format=plain``; archive extension ``.pgsql.gz`` +- **MySQL / MariaDB** – dumped via ``mysqldump --single-transaction``; archive extension ``.mysql.gz`` """ import gzip import hashlib import logging import os +import subprocess from datetime import datetime, timezone from pathlib import Path @@ -42,6 +49,13 @@ _BACKUP_TYPE_RETAIN: dict[str, str] = { "weekly": "backup_retain_weekly", } +#: Map of backend name → archive file extension. +_BACKEND_EXTENSIONS: dict[str, str] = { + "sqlite": ".db.gz", + "postgresql": ".pgsql.gz", + "mysql": ".mysql.gz", +} + def _backup_dir() -> Path: """Return (and create) the local backup directory.""" @@ -51,6 +65,14 @@ def _backup_dir() -> Path: return path +def _db_backend() -> str: + """Return the database backend name (e.g. ``'sqlite'``, ``'postgresql'``, ``'mysql'``).""" + from sqlalchemy.engine.url import make_url + + url = make_url(settings.database_url) + return url.get_backend_name() + + def _db_path() -> Path | None: """Return the SQLite database file path, or None for non-SQLite databases.""" from sqlalchemy.engine.url import make_url @@ -64,6 +86,20 @@ def _db_path() -> Path | None: return Path(db) +def _archive_ext_for_backend(backend: str) -> str: + """Return the archive file extension for the given database backend. + + Args: + backend: Backend name as returned by + ``sqlalchemy.engine.url.URL.get_backend_name()`` (e.g. ``'sqlite'``). + + Returns: + File extension string including the leading dot, e.g. ``'.db.gz'``. + Falls back to ``'.sql.gz'`` for unknown backends. + """ + return _BACKEND_EXTENSIONS.get(backend, ".sql.gz") + + def _sha256(path: Path) -> str: """Return the SHA-256 hex digest of *path*.""" h = hashlib.sha256() @@ -86,6 +122,277 @@ def _dump_sqlite(db_path: Path, dest: Path) -> None: conn.close() +def _dump_postgresql(db_url: str, dest: Path) -> None: + """Write a gzip-compressed ``pg_dump`` of the PostgreSQL database to *dest*. + + Uses ``PGPASSWORD`` environment variable so the password is never exposed on + the process command line. + + Args: + db_url: Full SQLAlchemy database URL (e.g. ``postgresql://user:pass@host/db``). + dest: Destination path for the ``.pgsql.gz`` archive. + + Raises: + RuntimeError: If ``pg_dump`` exits with a non-zero return code. + FileNotFoundError: If the ``pg_dump`` binary is not found. + """ + from sqlalchemy.engine.url import make_url + + url = make_url(db_url) + env = os.environ.copy() + if url.password: + env["PGPASSWORD"] = str(url.password) + + # Command arguments are built from the SQLAlchemy URL (admin-configured DATABASE_URL), + # not from user-controlled input. shell=False (the default when passing a list) is used + # so there is no shell interpretation of the argument values. + cmd: list[str] = ["pg_dump", "--format=plain", "--no-password"] + if url.host: + cmd.extend(["-h", url.host]) + if url.port: + cmd.extend(["-p", str(url.port)]) + if url.username: + cmd.extend(["-U", url.username]) + if url.database: + cmd.append(url.database) + + with gzip.open(str(dest), "wb") as gz: + proc = subprocess.Popen( # noqa: S603 + cmd, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + env=env, + ) + stdout = proc.stdout + if stdout is None: # pragma: no cover – guaranteed by stdout=PIPE + raise RuntimeError("pg_dump produced no stdout pipe") + try: + while True: + chunk = stdout.read(65536) + if not chunk: + break + gz.write(chunk) + finally: + stdout.close() + stderr_bytes = proc.stderr.read() if proc.stderr else b"" + proc.wait() + + if proc.returncode != 0: + dest.unlink(missing_ok=True) + raise RuntimeError( + f"pg_dump exited with code {proc.returncode}: {stderr_bytes.decode(errors='replace').strip()}" + ) + + +def _dump_mysql(db_url: str, dest: Path) -> None: + """Write a gzip-compressed ``mysqldump`` of the MySQL database to *dest*. + + Uses the ``MYSQL_PWD`` environment variable so the password is never exposed + on the process command line. + + Args: + db_url: Full SQLAlchemy database URL + (e.g. ``mysql+pymysql://user:pass@host/db``). + dest: Destination path for the ``.mysql.gz`` archive. + + Raises: + RuntimeError: If ``mysqldump`` exits with a non-zero return code. + FileNotFoundError: If the ``mysqldump`` binary is not found. + """ + from sqlalchemy.engine.url import make_url + + url = make_url(db_url) + env = os.environ.copy() + if url.password: + env["MYSQL_PWD"] = str(url.password) + + # Command arguments are built from the SQLAlchemy URL (admin-configured DATABASE_URL). + # shell=False (list form) prevents shell interpretation of argument values. + cmd: list[str] = ["mysqldump", "--single-transaction", "--routines", "--triggers"] + if url.host: + cmd.extend(["-h", url.host]) + if url.port: + cmd.extend(["-P", str(url.port)]) + if url.username: + cmd.extend(["-u", url.username]) + if url.database: + cmd.append(url.database) + + with gzip.open(str(dest), "wb") as gz: + proc = subprocess.Popen( # noqa: S603 + cmd, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + env=env, + ) + stdout = proc.stdout + if stdout is None: # pragma: no cover – guaranteed by stdout=PIPE + raise RuntimeError("mysqldump produced no stdout pipe") + try: + while True: + chunk = stdout.read(65536) + if not chunk: + break + gz.write(chunk) + finally: + stdout.close() + stderr_bytes = proc.stderr.read() if proc.stderr else b"" + proc.wait() + + if proc.returncode != 0: + dest.unlink(missing_ok=True) + raise RuntimeError( + f"mysqldump exited with code {proc.returncode}: {stderr_bytes.decode(errors='replace').strip()}" + ) + + +def _restore_sqlite(db_path: Path, archive_path: Path) -> None: + """Restore a SQLite database from a gzip-compressed SQL dump archive. + + Validates the SQL by replaying it on an in-memory database before touching + the live file. Saves a ``.pre_restore`` rollback copy first. + + Args: + db_path: Path to the live SQLite database file to overwrite. + archive_path: Path to the ``.db.gz`` gzip-compressed SQL dump. + + Raises: + ValueError: If the archive cannot be decompressed or contains invalid SQL. + RuntimeError: If writing the restored database fails. + """ + import shutil + import sqlite3 + + # Decompress and read SQL statements + try: + with gzip.open(str(archive_path), "rt", encoding="utf-8") as gz: + sql_script = gz.read() + except Exception as exc: + raise ValueError(f"Failed to decompress backup file: {exc}") from exc + + # Validate by replaying on an in-memory database + try: + mem_conn = sqlite3.connect(":memory:") + mem_conn.executescript(sql_script) + mem_conn.close() + except sqlite3.Error as exc: + raise ValueError(f"Backup file contains invalid SQL: {exc}") from exc + + # Preserve the current DB before overwriting + bak = str(db_path) + ".pre_restore" + try: + shutil.copy2(str(db_path), bak) + except OSError as exc: + logger.warning(f"Could not create pre-restore backup at {bak}: {exc}") + + try: + restore_conn = sqlite3.connect(str(db_path)) + restore_conn.executescript(sql_script) + restore_conn.close() + except sqlite3.Error as exc: + # Attempt rollback to the pre-restore copy + try: + if os.path.exists(bak): + shutil.copy2(bak, str(db_path)) + except OSError as rollback_exc: + logger.error(f"Rollback failed; database may be corrupted: {rollback_exc}") + raise RuntimeError(f"SQLite restore failed: {exc}") from exc + + +def _restore_postgresql(db_url: str, archive_path: Path) -> None: + """Restore a PostgreSQL database from a gzip-compressed SQL dump archive. + + Pipes the decompressed dump to ``psql``. Uses ``PGPASSWORD`` so the + password is never exposed on the process command line. + + Args: + db_url: Full SQLAlchemy database URL. + archive_path: Path to the ``.pgsql.gz`` gzip-compressed ``pg_dump`` archive. + + Raises: + RuntimeError: If ``psql`` exits with a non-zero return code. + FileNotFoundError: If the ``psql`` binary is not found. + """ + from sqlalchemy.engine.url import make_url + + url = make_url(db_url) + env = os.environ.copy() + if url.password: + env["PGPASSWORD"] = str(url.password) + + # Command arguments are built from the SQLAlchemy URL (admin-configured DATABASE_URL). + # shell=False (list form) prevents shell interpretation of argument values. + cmd: list[str] = ["psql", "--no-password"] + if url.host: + cmd.extend(["-h", url.host]) + if url.port: + cmd.extend(["-p", str(url.port)]) + if url.username: + cmd.extend(["-U", url.username]) + if url.database: + cmd.append(url.database) + + with gzip.open(str(archive_path), "rb") as gz: + proc = subprocess.Popen( # noqa: S603 + cmd, + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + env=env, + ) + _, stderr_bytes = proc.communicate(input=gz.read()) + + if proc.returncode != 0: + raise RuntimeError(f"psql exited with code {proc.returncode}: {stderr_bytes.decode(errors='replace').strip()}") + + +def _restore_mysql(db_url: str, archive_path: Path) -> None: + """Restore a MySQL database from a gzip-compressed SQL dump archive. + + Pipes the decompressed dump to ``mysql``. Uses the ``MYSQL_PWD`` + environment variable so the password is never exposed on the command line. + + Args: + db_url: Full SQLAlchemy database URL. + archive_path: Path to the ``.mysql.gz`` gzip-compressed ``mysqldump`` archive. + + Raises: + RuntimeError: If ``mysql`` exits with a non-zero return code. + FileNotFoundError: If the ``mysql`` binary is not found. + """ + from sqlalchemy.engine.url import make_url + + url = make_url(db_url) + env = os.environ.copy() + if url.password: + env["MYSQL_PWD"] = str(url.password) + + # Command arguments are built from the SQLAlchemy URL (admin-configured DATABASE_URL). + # shell=False (list form) prevents shell interpretation of argument values. + cmd: list[str] = ["mysql"] + if url.host: + cmd.extend(["-h", url.host]) + if url.port: + cmd.extend(["-P", str(url.port)]) + if url.username: + cmd.extend(["-u", url.username]) + if url.database: + cmd.append(url.database) + + with gzip.open(str(archive_path), "rb") as gz: + proc = subprocess.Popen( # noqa: S603 + cmd, + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + env=env, + ) + _, stderr_bytes = proc.communicate(input=gz.read()) + + if proc.returncode != 0: + raise RuntimeError(f"mysql exited with code {proc.returncode}: {stderr_bytes.decode(errors='replace').strip()}") + + def _apply_retention(backup_type: str, db: object) -> None: """Delete local backups beyond the retention limit for *backup_type*. @@ -314,6 +621,11 @@ def _email_backup(archive_path: Path, filename: str) -> None: def create_backup(self, backup_type: str = "hourly") -> dict: """Create a database backup archive and apply retention. + Supports SQLite (``.db.gz``), PostgreSQL (``.pgsql.gz``), and + MySQL / MariaDB (``.mysql.gz``) databases. The native dump tool for the + configured backend (``sqlite3``, ``pg_dump``, or ``mysqldump``) must be + available on the worker's ``PATH``. + Args: backup_type: ``"hourly"``, ``"daily"``, or ``"weekly"``. @@ -327,19 +639,27 @@ def create_backup(self, backup_type: str = "hourly") -> dict: logger.debug("Backup is disabled; skipping create_backup task.") return {"status": "disabled"} + backend = _db_backend() + ext = _archive_ext_for_backend(backend) + ts = datetime.now(timezone.utc).strftime("%Y-%m-%dT%H-%M-%S") - filename = f"backup_{backup_type}_{ts}.db.gz" + filename = f"backup_{backup_type}_{ts}{ext}" archive_path = _backup_dir() / filename - db_path = _db_path() - if db_path is None: - logger.warning("Backup task skipped: non-SQLite databases are not supported for file-based backups.") + # SQLite: verify the database file exists before attempting to dump it + db_path: Path | None = None + if backend == "sqlite": + db_path = _db_path() + if db_path is None: + logger.warning("Backup task skipped: in-memory SQLite databases are not supported.") + return {"status": "unsupported_db"} + if not db_path.exists(): + logger.error(f"Database file not found: {db_path}") + return {"status": "error", "detail": f"DB file missing: {db_path}"} + elif backend not in ("postgresql", "mysql"): + logger.warning(f"Backup task skipped: unsupported database backend '{backend}'.") return {"status": "unsupported_db"} - if not db_path.exists(): - logger.error(f"Database file not found: {db_path}") - return {"status": "error", "detail": f"DB file missing: {db_path}"} - status = "ok" checksum: str | None = None size_bytes = 0 @@ -347,7 +667,15 @@ def create_backup(self, backup_type: str = "hourly") -> dict: remote_path: str | None = None try: - _dump_sqlite(db_path, archive_path) + if backend == "sqlite": + # db_path is guaranteed non-None: we returned early if it were None + if db_path is None: # pragma: no cover + return {"status": "error", "detail": "db_path unexpectedly None"} + _dump_sqlite(db_path, archive_path) + elif backend == "postgresql": + _dump_postgresql(settings.database_url, archive_path) + elif backend == "mysql": + _dump_mysql(settings.database_url, archive_path) size_bytes = archive_path.stat().st_size checksum = _sha256(archive_path) logger.info(f"Created {backup_type} backup: {archive_path} ({size_bytes:,} bytes)") diff --git a/docs/ConfigurationGuide.md b/docs/ConfigurationGuide.md index 2de94abd..3ea63e1e 100644 --- a/docs/ConfigurationGuide.md +++ b/docs/ConfigurationGuide.md @@ -1037,9 +1037,13 @@ Webhook URLs, secrets, and subscribed events are configured per-webhook via the ### Backup & Restore -DocuElevate can automatically back up the SQLite database on a scheduled basis. +DocuElevate automatically backs up the database on a scheduled basis. Backups are managed from the **Admin → Backup & Restore** dashboard. +Supported database backends: **SQLite** (`.db.gz`), **PostgreSQL** (`.pgsql.gz`), **MySQL / MariaDB** (`.mysql.gz`). +For PostgreSQL and MySQL backups the respective CLI client (`pg_dump` / `psql` or `mysqldump` / `mysql`) must be installed on the Celery worker host. +See the [Database Configuration Guide](DatabaseConfiguration.md#backup-procedures) for setup details. + | **Variable** | **Description** | **Default** | |--------------------------------|-----------------------------------------------------------------------------------------------|---------------------| | `BACKUP_ENABLED` | Enable or disable automatic scheduled backups (`True`/`False`). | `True` | diff --git a/docs/DatabaseConfiguration.md b/docs/DatabaseConfiguration.md index 0e5811aa..9eb1fece 100644 --- a/docs/DatabaseConfiguration.md +++ b/docs/DatabaseConfiguration.md @@ -341,29 +341,91 @@ Disable `prepared_statements` when using PgBouncer in transaction mode. ## Backup Procedures -### PostgreSQL +DocuElevate's built-in **Backup & Restore** feature (Admin → Backup & Restore) supports all three +database backends natively, using the native dump tools of each database. -**Manual backup:** +| Backend | Backup tool | Archive extension | Restore tool | +|----------------|--------------|-------------------|--------------| +| SQLite | `sqlite3` (built-in Python) | `.db.gz` | `sqlite3` (built-in Python) | +| PostgreSQL | `pg_dump` | `.pgsql.gz` | `psql` | +| MySQL/MariaDB | `mysqldump` | `.mysql.gz` | `mysql` | + +Passwords are passed via the `PGPASSWORD` (PostgreSQL) and `MYSQL_PWD` (MySQL) environment +variables so they are never exposed on the process command line. + +### Prerequisites + +For PostgreSQL and MySQL backups the corresponding CLI client must be installed on the +worker host (the container / server that runs Celery workers): + +```bash +# PostgreSQL clients (Debian/Ubuntu) +apt-get install -y postgresql-client + +# MySQL clients (Debian/Ubuntu) +apt-get install -y default-mysql-client +``` + +The binaries required are: + +- **PostgreSQL**: `pg_dump` (backup) and `psql` (restore) +- **MySQL / MariaDB**: `mysqldump` (backup) and `mysql` (restore) + +### Using the Admin Dashboard + +Navigate to **Admin → Backup & Restore** to: + +- Trigger manual backups (hourly / daily / weekly) +- Download backup archives +- Upload and restore a backup archive +- Configure retention and remote storage destinations + +### PostgreSQL – manual backup/restore + +**Manual backup using DocuElevate's archive format (for use with the UI restore):** + +```bash +pg_dump --format=plain --no-password \ + -h localhost -U docuelevate docuelevate \ + | gzip > docuelevate_$(date +%Y%m%d_%H%M).pgsql.gz +``` + +**Restore via the DocuElevate UI:** upload the `.pgsql.gz` file on the Backup & Restore page. + +**Manual restore using native tools (custom format):** ```bash pg_dump -h localhost -U docuelevate -F c docuelevate > docuelevate_$(date +%Y%m%d_%H%M).dump -``` - -**Restore:** - -```bash pg_restore -h localhost -U docuelevate -d docuelevate docuelevate_20240101_1200.dump ``` **Automated daily backup (cron example):** ```cron -0 2 * * * pg_dump -h localhost -U docuelevate -F c docuelevate | gzip > /backups/docuelevate_$(date +\%Y\%m\%d).dump.gz +0 2 * * * pg_dump --format=plain -h localhost -U docuelevate docuelevate | gzip > /backups/docuelevate_$(date +\%Y\%m\%d).pgsql.gz ``` Use your cloud provider's automated backup feature when available (e.g., RDS automated snapshots, Cloud SQL backups). -### SQLite +### MySQL / MariaDB – manual backup/restore + +**Manual backup using DocuElevate's archive format (for use with the UI restore):** + +```bash +MYSQL_PWD=yourpassword mysqldump --single-transaction --routines --triggers \ + -h localhost -u docuelevate docuelevate \ + | gzip > docuelevate_$(date +%Y%m%d_%H%M).mysql.gz +``` + +**Restore via the DocuElevate UI:** upload the `.mysql.gz` file on the Backup & Restore page. + +**Manual restore using native tools:** + +```bash +gunzip -c docuelevate_20240101_1200.mysql.gz | mysql -h localhost -u docuelevate -p docuelevate +``` + +### SQLite – manual backup/restore ```bash # Stop the application first, or use SQLite's online backup API diff --git a/tests/test_backup.py b/tests/test_backup.py index d4859493..34dd115b 100644 --- a/tests/test_backup.py +++ b/tests/test_backup.py @@ -236,6 +236,254 @@ class TestBackupTaskHelpers: result = _db_path() assert result is None + def test_db_backend_sqlite(self): + """_db_backend() returns 'sqlite' for SQLite URLs.""" + from app.tasks.backup_tasks import _db_backend + + with patch("app.tasks.backup_tasks.settings") as mock_settings: + mock_settings.database_url = "sqlite:////tmp/test.db" + assert _db_backend() == "sqlite" + + def test_db_backend_postgresql(self): + """_db_backend() returns 'postgresql' for PostgreSQL URLs.""" + from app.tasks.backup_tasks import _db_backend + + with patch("app.tasks.backup_tasks.settings") as mock_settings: + mock_settings.database_url = "postgresql://user:pass@localhost/db" + assert _db_backend() == "postgresql" + + def test_db_backend_mysql(self): + """_db_backend() returns 'mysql' for MySQL URLs.""" + from app.tasks.backup_tasks import _db_backend + + with patch("app.tasks.backup_tasks.settings") as mock_settings: + mock_settings.database_url = "mysql+pymysql://user:pass@localhost/db" + assert _db_backend() == "mysql" + + def test_archive_ext_sqlite(self): + """_archive_ext_for_backend() returns '.db.gz' for sqlite.""" + from app.tasks.backup_tasks import _archive_ext_for_backend + + assert _archive_ext_for_backend("sqlite") == ".db.gz" + + def test_archive_ext_postgresql(self): + """_archive_ext_for_backend() returns '.pgsql.gz' for postgresql.""" + from app.tasks.backup_tasks import _archive_ext_for_backend + + assert _archive_ext_for_backend("postgresql") == ".pgsql.gz" + + def test_archive_ext_mysql(self): + """_archive_ext_for_backend() returns '.mysql.gz' for mysql.""" + from app.tasks.backup_tasks import _archive_ext_for_backend + + assert _archive_ext_for_backend("mysql") == ".mysql.gz" + + def test_archive_ext_unknown(self): + """_archive_ext_for_backend() falls back to '.sql.gz' for unknown backends.""" + from app.tasks.backup_tasks import _archive_ext_for_backend + + assert _archive_ext_for_backend("mssql") == ".sql.gz" + + def test_dump_postgresql_success(self, tmp_path): + """_dump_postgresql() streams pg_dump output into a gzip archive.""" + from unittest.mock import MagicMock + + from app.tasks.backup_tasks import _dump_postgresql + + dest = tmp_path / "dump.pgsql.gz" + fake_sql = b"-- PostgreSQL database dump\nSELECT 1;\n" + + mock_proc = MagicMock() + mock_proc.stdout.read.side_effect = [fake_sql, b""] + mock_proc.stderr.read.return_value = b"" + mock_proc.returncode = 0 + + with patch("app.tasks.backup_tasks.subprocess.Popen", return_value=mock_proc): + _dump_postgresql("postgresql://user:pass@localhost/testdb", dest) + + assert dest.exists() + with gzip.open(str(dest), "rb") as gz: + assert gz.read() == fake_sql + + def test_dump_postgresql_failure(self, tmp_path): + """_dump_postgresql() raises RuntimeError when pg_dump fails.""" + from unittest.mock import MagicMock + + from app.tasks.backup_tasks import _dump_postgresql + + dest = tmp_path / "dump.pgsql.gz" + + mock_proc = MagicMock() + mock_proc.stdout.read.side_effect = [b""] + mock_proc.stderr.read.return_value = b"FATAL: connection refused" + mock_proc.returncode = 1 + + with patch("app.tasks.backup_tasks.subprocess.Popen", return_value=mock_proc): + with pytest.raises(RuntimeError, match="pg_dump exited with code 1"): + _dump_postgresql("postgresql://user:pass@localhost/testdb", dest) + + def test_dump_mysql_success(self, tmp_path): + """_dump_mysql() streams mysqldump output into a gzip archive.""" + from unittest.mock import MagicMock + + from app.tasks.backup_tasks import _dump_mysql + + dest = tmp_path / "dump.mysql.gz" + fake_sql = b"-- MySQL dump\nCREATE TABLE t (id INT);\n" + + mock_proc = MagicMock() + mock_proc.stdout.read.side_effect = [fake_sql, b""] + mock_proc.stderr.read.return_value = b"" + mock_proc.returncode = 0 + + with patch("app.tasks.backup_tasks.subprocess.Popen", return_value=mock_proc): + _dump_mysql("mysql+pymysql://user:pass@localhost/testdb", dest) + + assert dest.exists() + with gzip.open(str(dest), "rb") as gz: + assert gz.read() == fake_sql + + def test_dump_mysql_failure(self, tmp_path): + """_dump_mysql() raises RuntimeError when mysqldump fails.""" + from unittest.mock import MagicMock + + from app.tasks.backup_tasks import _dump_mysql + + dest = tmp_path / "dump.mysql.gz" + + mock_proc = MagicMock() + mock_proc.stdout.read.side_effect = [b""] + mock_proc.stderr.read.return_value = b"ERROR: Access denied" + mock_proc.returncode = 1 + + with patch("app.tasks.backup_tasks.subprocess.Popen", return_value=mock_proc): + with pytest.raises(RuntimeError, match="mysqldump exited with code 1"): + _dump_mysql("mysql+pymysql://user:pass@localhost/testdb", dest) + + def test_restore_sqlite_success(self, tmp_path): + """_restore_sqlite() applies a valid SQL dump to a SQLite file.""" + from app.tasks.backup_tasks import _restore_sqlite + + db_file = tmp_path / "test.db" + conn = sqlite3.connect(str(db_file)) + conn.execute("CREATE TABLE old (id INTEGER)") + conn.commit() + conn.close() + + # Create a valid dump archive + sql = "BEGIN TRANSACTION;\nCREATE TABLE new_tbl (x TEXT);\nCOMMIT;\n" + archive = tmp_path / "dump.db.gz" + with gzip.open(str(archive), "wt") as gz: + gz.write(sql) + + _restore_sqlite(db_file, archive) + + conn2 = sqlite3.connect(str(db_file)) + tables = [r[0] for r in conn2.execute("SELECT name FROM sqlite_master WHERE type='table'")] + conn2.close() + assert "new_tbl" in tables + + def test_restore_sqlite_invalid_gz(self, tmp_path): + """_restore_sqlite() raises ValueError for corrupt gzip content.""" + from app.tasks.backup_tasks import _restore_sqlite + + db_file = tmp_path / "test.db" + db_file.write_bytes(b"") + archive = tmp_path / "bad.db.gz" + archive.write_bytes(b"not gzip data") + + with pytest.raises(ValueError, match="Failed to decompress"): + _restore_sqlite(db_file, archive) + + def test_restore_sqlite_invalid_sql(self, tmp_path): + """_restore_sqlite() raises ValueError for invalid SQL content.""" + from app.tasks.backup_tasks import _restore_sqlite + + db_file = tmp_path / "test.db" + db_file.write_bytes(b"") + archive = tmp_path / "bad.db.gz" + with gzip.open(str(archive), "wt") as gz: + gz.write("THIS IS NOT VALID SQL!!!;\n") + + with pytest.raises(ValueError, match="invalid SQL"): + _restore_sqlite(db_file, archive) + + def test_restore_postgresql_success(self, tmp_path): + """_restore_postgresql() pipes the archive to psql.""" + from unittest.mock import MagicMock + + from app.tasks.backup_tasks import _restore_postgresql + + fake_sql = b"-- PostgreSQL dump\nSELECT 1;\n" + archive = tmp_path / "dump.pgsql.gz" + with gzip.open(str(archive), "wb") as gz: + gz.write(fake_sql) + + mock_proc = MagicMock() + mock_proc.communicate.return_value = (b"", b"") + mock_proc.returncode = 0 + + with patch("app.tasks.backup_tasks.subprocess.Popen", return_value=mock_proc): + _restore_postgresql("postgresql://user:pass@localhost/testdb", archive) + + mock_proc.communicate.assert_called_once_with(input=fake_sql) + + def test_restore_postgresql_failure(self, tmp_path): + """_restore_postgresql() raises RuntimeError when psql fails.""" + from unittest.mock import MagicMock + + from app.tasks.backup_tasks import _restore_postgresql + + archive = tmp_path / "dump.pgsql.gz" + with gzip.open(str(archive), "wb") as gz: + gz.write(b"SELECT 1;") + + mock_proc = MagicMock() + mock_proc.communicate.return_value = (b"", b"ERROR: invalid input") + mock_proc.returncode = 1 + + with patch("app.tasks.backup_tasks.subprocess.Popen", return_value=mock_proc): + with pytest.raises(RuntimeError, match="psql exited with code 1"): + _restore_postgresql("postgresql://user:pass@localhost/testdb", archive) + + def test_restore_mysql_success(self, tmp_path): + """_restore_mysql() pipes the archive to mysql.""" + from unittest.mock import MagicMock + + from app.tasks.backup_tasks import _restore_mysql + + fake_sql = b"-- MySQL dump\nSELECT 1;\n" + archive = tmp_path / "dump.mysql.gz" + with gzip.open(str(archive), "wb") as gz: + gz.write(fake_sql) + + mock_proc = MagicMock() + mock_proc.communicate.return_value = (b"", b"") + mock_proc.returncode = 0 + + with patch("app.tasks.backup_tasks.subprocess.Popen", return_value=mock_proc): + _restore_mysql("mysql+pymysql://user:pass@localhost/testdb", archive) + + mock_proc.communicate.assert_called_once_with(input=fake_sql) + + def test_restore_mysql_failure(self, tmp_path): + """_restore_mysql() raises RuntimeError when mysql fails.""" + from unittest.mock import MagicMock + + from app.tasks.backup_tasks import _restore_mysql + + archive = tmp_path / "dump.mysql.gz" + with gzip.open(str(archive), "wb") as gz: + gz.write(b"SELECT 1;") + + mock_proc = MagicMock() + mock_proc.communicate.return_value = (b"", b"ERROR: Access denied") + mock_proc.returncode = 1 + + with patch("app.tasks.backup_tasks.subprocess.Popen", return_value=mock_proc): + with pytest.raises(RuntimeError, match="mysql exited with code 1"): + _restore_mysql("mysql+pymysql://user:pass@localhost/testdb", archive) + def test_apply_retention_prunes_old(self, tmp_path, db_session): """_apply_retention() deletes backups beyond the retention limit.""" from app.tasks.backup_tasks import _apply_retention @@ -352,20 +600,28 @@ class TestCreateBackupTask: result = create_backup("hourly") assert result["status"] == "disabled" - def test_non_sqlite_db(self): - """create_backup returns unsupported_db for non-SQLite databases.""" + def test_unsupported_db_backend(self): + """create_backup returns unsupported_db for backends other than sqlite/postgresql/mysql.""" from app.tasks.backup_tasks import create_backup - with ( - patch("app.tasks.backup_tasks.settings") as mock_settings, - patch("app.tasks.backup_tasks._db_path", return_value=None), - ): + with patch("app.tasks.backup_tasks.settings") as mock_settings: mock_settings.backup_enabled = True + mock_settings.database_url = "mssql+pyodbc://user:pass@server/db" + result = create_backup("hourly") + assert result["status"] == "unsupported_db" + + def test_in_memory_sqlite_unsupported(self): + """create_backup returns unsupported_db for in-memory SQLite.""" + from app.tasks.backup_tasks import create_backup + + with patch("app.tasks.backup_tasks.settings") as mock_settings: + mock_settings.backup_enabled = True + mock_settings.database_url = "sqlite:///:memory:" result = create_backup("hourly") assert result["status"] == "unsupported_db" def test_missing_db_file(self, tmp_path): - """create_backup returns error when the DB file does not exist.""" + """create_backup returns error when the SQLite DB file does not exist.""" from app.tasks.backup_tasks import create_backup missing = tmp_path / "does_not_exist.db" @@ -375,6 +631,7 @@ class TestCreateBackupTask: patch("app.tasks.backup_tasks._db_path", return_value=missing), ): mock_settings.backup_enabled = True + mock_settings.database_url = f"sqlite:///{missing}" result = create_backup("hourly") assert result["status"] == "error" @@ -400,6 +657,7 @@ class TestCreateBackupTask: ): backup_dir.mkdir(parents=True, exist_ok=True) mock_settings.backup_enabled = True + mock_settings.database_url = f"sqlite:///{db_file}" mock_db = MagicMock() mock_sl.return_value.__enter__ = MagicMock(return_value=mock_db) mock_sl.return_value.__exit__ = MagicMock(return_value=False) @@ -408,6 +666,69 @@ class TestCreateBackupTask: assert result["status"] == "ok" assert "filename" in result assert result["filename"].startswith("backup_hourly_") + assert result["filename"].endswith(".db.gz") + + def test_successful_backup_postgresql(self, tmp_path): + """create_backup creates a .pgsql.gz archive for PostgreSQL databases.""" + from app.tasks.backup_tasks import create_backup + + backup_dir = tmp_path / "backups" + backup_dir.mkdir() + + def fake_pg_dump(db_url: str, dest: Path) -> None: + with gzip.open(str(dest), "wb") as gz: + gz.write(b"-- PostgreSQL dump\n") + + with ( + patch("app.tasks.backup_tasks.settings") as mock_settings, + patch("app.tasks.backup_tasks._backup_dir", return_value=backup_dir), + patch("app.tasks.backup_tasks._dump_postgresql", side_effect=fake_pg_dump), + patch("app.tasks.backup_tasks._upload_remote", return_value=None), + patch("app.tasks.backup_tasks._apply_retention"), + patch("app.tasks.backup_tasks._prune_remote_backups"), + patch("app.tasks.backup_tasks.SessionLocal") as mock_sl, + ): + mock_settings.backup_enabled = True + mock_settings.database_url = "postgresql://user:pass@localhost/testdb" + mock_db = MagicMock() + mock_sl.return_value.__enter__ = MagicMock(return_value=mock_db) + mock_sl.return_value.__exit__ = MagicMock(return_value=False) + result = create_backup("daily") + + assert result["status"] == "ok" + assert result["filename"].endswith(".pgsql.gz") + assert "daily" in result["filename"] + + def test_successful_backup_mysql(self, tmp_path): + """create_backup creates a .mysql.gz archive for MySQL databases.""" + from app.tasks.backup_tasks import create_backup + + backup_dir = tmp_path / "backups" + backup_dir.mkdir() + + def fake_mysql_dump(db_url: str, dest: Path) -> None: + with gzip.open(str(dest), "wb") as gz: + gz.write(b"-- MySQL dump\n") + + with ( + patch("app.tasks.backup_tasks.settings") as mock_settings, + patch("app.tasks.backup_tasks._backup_dir", return_value=backup_dir), + patch("app.tasks.backup_tasks._dump_mysql", side_effect=fake_mysql_dump), + patch("app.tasks.backup_tasks._upload_remote", return_value=None), + patch("app.tasks.backup_tasks._apply_retention"), + patch("app.tasks.backup_tasks._prune_remote_backups"), + patch("app.tasks.backup_tasks.SessionLocal") as mock_sl, + ): + mock_settings.backup_enabled = True + mock_settings.database_url = "mysql+pymysql://user:pass@localhost/testdb" + mock_db = MagicMock() + mock_sl.return_value.__enter__ = MagicMock(return_value=mock_db) + mock_sl.return_value.__exit__ = MagicMock(return_value=False) + result = create_backup("weekly") + + assert result["status"] == "ok" + assert result["filename"].endswith(".mysql.gz") + assert "weekly" in result["filename"] def test_invalid_backup_type_defaults_to_hourly(self, tmp_path): """create_backup normalises unknown backup_type to 'hourly'.""" @@ -431,6 +752,7 @@ class TestCreateBackupTask: patch("app.tasks.backup_tasks.SessionLocal") as mock_sl, ): mock_settings.backup_enabled = True + mock_settings.database_url = f"sqlite:///{db_file}" mock_db = MagicMock() mock_sl.return_value.__enter__ = MagicMock(return_value=mock_db) mock_sl.return_value.__exit__ = MagicMock(return_value=False) @@ -549,19 +871,24 @@ class TestBackupAPIEndpoints: assert resp.status_code == 403 def test_restore_wrong_extension(self, admin_client): - """POST /api/admin/backup/restore rejects non-.db.gz files.""" + """POST /api/admin/backup/restore rejects files with wrong extension for current backend.""" + # Default test env uses sqlite:///:memory: → expects .db.gz resp = admin_client.post( "/api/admin/backup/restore", files={"file": ("backup.zip", b"data", "application/zip")}, ) assert resp.status_code == 400 - def test_restore_invalid_gz_content(self, admin_client): + def test_restore_invalid_gz_content(self, admin_client, tmp_path): """POST /api/admin/backup/restore rejects corrupt gzip data.""" - resp = admin_client.post( - "/api/admin/backup/restore", - files={"file": ("backup.db.gz", b"not gzip data at all", "application/gzip")}, - ) + db_file = tmp_path / "test.db" + db_file.write_bytes(b"") + + with patch("app.tasks.backup_tasks._db_path", return_value=db_file): + resp = admin_client.post( + "/api/admin/backup/restore", + files={"file": ("backup.db.gz", b"not gzip data at all", "application/gzip")}, + ) assert resp.status_code == 400 def test_restore_valid_archive(self, admin_client, tmp_path): @@ -581,11 +908,11 @@ class TestBackupAPIEndpoints: assert resp.status_code == 200 assert resp.json()["status"] == "restored" - def test_restore_non_sqlite_db(self, admin_client): - """POST /api/admin/backup/restore returns 400 for non-SQLite database.""" - sql = "BEGIN TRANSACTION;\nCOMMIT;\n" - gz_data = gzip.compress(sql.encode()) + def test_restore_in_memory_sqlite(self, admin_client): + """POST /api/admin/backup/restore returns 400 for in-memory SQLite (no file to restore to).""" + gz_data = gzip.compress(b"BEGIN TRANSACTION;\nCOMMIT;\n") + # _db_path() returns None for :memory: URLs → 400 with patch("app.tasks.backup_tasks._db_path", return_value=None): resp = admin_client.post( "/api/admin/backup/restore", @@ -593,6 +920,106 @@ class TestBackupAPIEndpoints: ) assert resp.status_code == 400 + def test_restore_wrong_extension_for_postgresql(self, admin_client): + """POST /api/admin/backup/restore returns 400 when uploading .db.gz for PostgreSQL backend.""" + gz_data = gzip.compress(b"-- PostgreSQL dump") + + with patch("app.config.settings") as mock_settings: + mock_settings.database_url = "postgresql://user:pass@localhost/testdb" + resp = admin_client.post( + "/api/admin/backup/restore", + files={"file": ("backup.db.gz", gz_data, "application/gzip")}, + ) + assert resp.status_code == 400 + + def test_restore_wrong_extension_for_mysql(self, admin_client): + """POST /api/admin/backup/restore returns 400 when uploading .db.gz for MySQL backend.""" + gz_data = gzip.compress(b"-- MySQL dump") + + with patch("app.config.settings") as mock_settings: + mock_settings.database_url = "mysql+pymysql://user:pass@localhost/testdb" + resp = admin_client.post( + "/api/admin/backup/restore", + files={"file": ("backup.db.gz", gz_data, "application/gzip")}, + ) + assert resp.status_code == 400 + + def test_restore_postgresql_success(self, admin_client): + """POST /api/admin/backup/restore succeeds for PostgreSQL database.""" + gz_data = gzip.compress(b"-- PostgreSQL dump\n") + + with ( + patch("app.config.settings") as mock_settings, + patch("app.tasks.backup_tasks._restore_postgresql") as mock_restore, + ): + mock_settings.database_url = "postgresql://user:pass@localhost/testdb" + resp = admin_client.post( + "/api/admin/backup/restore", + files={"file": ("backup.pgsql.gz", gz_data, "application/gzip")}, + ) + assert resp.status_code == 200 + assert resp.json()["status"] == "restored" + mock_restore.assert_called_once() + + def test_restore_mysql_success(self, admin_client): + """POST /api/admin/backup/restore succeeds for MySQL database.""" + gz_data = gzip.compress(b"-- MySQL dump\n") + + with ( + patch("app.config.settings") as mock_settings, + patch("app.tasks.backup_tasks._restore_mysql") as mock_restore, + ): + mock_settings.database_url = "mysql+pymysql://user:pass@localhost/testdb" + resp = admin_client.post( + "/api/admin/backup/restore", + files={"file": ("backup.mysql.gz", gz_data, "application/gzip")}, + ) + assert resp.status_code == 200 + assert resp.json()["status"] == "restored" + mock_restore.assert_called_once() + + def test_restore_postgresql_runtime_error(self, admin_client): + """POST /api/admin/backup/restore returns 500 when psql command fails.""" + gz_data = gzip.compress(b"-- PostgreSQL dump\n") + + with ( + patch("app.config.settings") as mock_settings, + patch("app.tasks.backup_tasks._restore_postgresql", side_effect=RuntimeError("psql failed")), + ): + mock_settings.database_url = "postgresql://user:pass@localhost/testdb" + resp = admin_client.post( + "/api/admin/backup/restore", + files={"file": ("backup.pgsql.gz", gz_data, "application/gzip")}, + ) + assert resp.status_code == 500 + + def test_restore_postgresql_missing_binary(self, admin_client): + """POST /api/admin/backup/restore returns 500 when psql binary is missing.""" + gz_data = gzip.compress(b"-- PostgreSQL dump\n") + + with ( + patch("app.config.settings") as mock_settings, + patch("app.tasks.backup_tasks._restore_postgresql", side_effect=FileNotFoundError("psql not found")), + ): + mock_settings.database_url = "postgresql://user:pass@localhost/testdb" + resp = admin_client.post( + "/api/admin/backup/restore", + files={"file": ("backup.pgsql.gz", gz_data, "application/gzip")}, + ) + assert resp.status_code == 500 + + def test_restore_unsupported_backend(self, admin_client): + """POST /api/admin/backup/restore returns 400 for an unsupported database backend.""" + gz_data = gzip.compress(b"-- some dump\n") + + with patch("app.config.settings") as mock_settings: + mock_settings.database_url = "mssql+pyodbc://user:pass@server/db" + resp = admin_client.post( + "/api/admin/backup/restore", + files={"file": ("backup.sql.gz", gz_data, "application/gzip")}, + ) + assert resp.status_code == 400 + # --------------------------------------------------------------------------- # View tests From d36ba88de765b688888c6e661256f8508da86d89 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Mar 2026 10:06:08 +0000 Subject: [PATCH 09/25] feat(auth): password reset, forgot username, and admin user management for local accounts - Add /forgot-password and /forgot-username page routes and templates - Update login page label to "Username or Email" (both already accepted by backend) - Add "Forgot password?" and "Forgot username?" links to login page - Add POST /api/auth/forgot-username endpoint + send_forgot_username_email() utility - Add admin endpoints: PATCH /local/{id}, POST /local/{id}/send-password-reset, POST /local/{id}/set-password - Update admin_users.html with Edit, Password, and Reset action buttons + modals - Add 23 tests; fix code review issues (import style, display_name clearing behaviour) - Update docs/API.md and docs/UserGuide.md Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> --- app/api/admin_users.py | 18 +- app/api/local_auth.py | 36 ++++ app/utils/local_auth.py | 35 ++++ docs/API.md | 125 +++++++++++ docs/UserGuide.md | 25 ++- frontend/templates/admin_users.html | 2 +- frontend/templates/forgot_username.html | 125 +++++++++++ frontend/templates/login.html | 11 +- tests/test_admin_users.py | 266 +++++++++++++++++++++++- tests/test_local_auth.py | 116 +++++++++++ 10 files changed, 742 insertions(+), 17 deletions(-) create mode 100644 frontend/templates/forgot_username.html diff --git a/app/api/admin_users.py b/app/api/admin_users.py index 1e5fc221..d73a1467 100644 --- a/app/api/admin_users.py +++ b/app/api/admin_users.py @@ -15,6 +15,7 @@ from pydantic import BaseModel, Field from sqlalchemy import func from sqlalchemy.orm import Session +from app.config import settings from app.database import get_db from app.models import FileRecord, LocalUser, UserProfile from app.utils.local_auth import generate_token, hash_password, send_password_reset_email @@ -393,7 +394,8 @@ def update_local_user(local_user_id: int, body: LocalUserUpdate, db: DbSession, user.email = body.email if body.display_name is not None: - user.display_name = body.display_name + # Normalise empty string to None so that clearing the field removes the display name + user.display_name = body.display_name or None if body.is_admin is not None: user.is_admin = body.is_admin @@ -431,9 +433,7 @@ def update_local_user(local_user_id: int, body: LocalUserUpdate, db: DbSession, status_code=status.HTTP_200_OK, summary="Send a password reset email to a local user", ) -def admin_send_password_reset( - local_user_id: int, request: Request, db: DbSession, _admin: AdminUser -) -> dict[str, Any]: +def admin_send_password_reset(local_user_id: int, request: Request, db: DbSession, _admin: AdminUser) -> dict[str, Any]: """Generate a password reset token and email the reset link to the local user. This is a last-resort tool for admins to help users who are locked out. @@ -443,13 +443,11 @@ def admin_send_password_reset( Raises: 404: Local user not found. """ - from app.config import settings as _settings - user = db.query(LocalUser).filter(LocalUser.id == local_user_id).first() if not user: raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Local user not found.") - if not _settings.email_host: + if not settings.email_host: logger.warning("Admin requested password reset for %s but SMTP is not configured", user.email) return {"sent": False, "reason": "SMTP is not configured on this server."} @@ -500,10 +498,10 @@ def admin_set_password( db.rollback() raise - logger.info( - "[SECURITY] ADMIN_SET_PASSWORD user=%s admin=%s", user.email, _admin.get("email", "unknown") - ) + logger.info("[SECURITY] ADMIN_SET_PASSWORD user=%s admin=%s", user.email, _admin.get("email", "unknown")) return {"updated": True, "email": user.email} + + def get_user(user_id: str, db: DbSession, _admin: AdminUser) -> dict[str, Any]: """Return profile and document statistics for a specific user.""" doc_count = db.query(func.count(FileRecord.id)).filter(FileRecord.owner_id == user_id).scalar() or 0 diff --git a/app/api/local_auth.py b/app/api/local_auth.py index f42cbe2b..be6df664 100644 --- a/app/api/local_auth.py +++ b/app/api/local_auth.py @@ -31,6 +31,7 @@ from app.utils.local_auth import ( generate_token, hash_password, is_token_expired, + send_forgot_username_email, send_password_reset_email, send_verification_email, ) @@ -79,6 +80,12 @@ class PasswordResetBody(BaseModel): new_password_confirm: str +class ForgotUsernameBody(BaseModel): + """Body for the forgot-username endpoint.""" + + email: str + + # --------------------------------------------------------------------------- # Page routes (return HTML) # --------------------------------------------------------------------------- @@ -107,6 +114,19 @@ async def verify_email_sent_page(request: Request) -> Any: return templates.TemplateResponse("verify_email_sent.html", {"request": request}) +@router.get("/forgot-username", include_in_schema=False) +async def forgot_username_page(request: Request) -> Any: + """Render the forgot-username page where users can request a username reminder email.""" + return templates.TemplateResponse( + "forgot_username.html", + { + "request": request, + "csrf_token": getattr(request.state, "csrf_token", ""), + "app_version": settings.version, + }, + ) + + @router.get("/forgot-password", include_in_schema=False) async def forgot_password_page(request: Request) -> Any: """Render the forgot-password page where users can request a reset email.""" @@ -350,3 +370,19 @@ async def reset_password(body: PasswordResetBody, db: DbSession) -> dict[str, st logger.info("[SECURITY] PASSWORD_RESET_SUCCESS user=%s", user.email) return {"message": "Password updated successfully."} + + +@router.post("/api/auth/forgot-username") +async def forgot_username(body: ForgotUsernameBody, db: DbSession) -> dict[str, str]: + """Send a username reminder email. + + Always returns 200 to avoid leaking whether an email is registered. + """ + user = db.query(LocalUser).filter(LocalUser.email == body.email).first() + if user: + try: + send_forgot_username_email(user.email, user.username) + except Exception as exc: + logger.warning("Failed to send forgot-username email to %s: %s", user.email, exc) + + return {"message": "Username reminder sent if account exists."} diff --git a/app/utils/local_auth.py b/app/utils/local_auth.py index 669d6819..20a54719 100644 --- a/app/utils/local_auth.py +++ b/app/utils/local_auth.py @@ -164,6 +164,41 @@ def send_password_reset_email(email: str, username: str, token: str, base_url: s _smtp_send(subject, html_body, plain_body, email) +def send_forgot_username_email(email: str, username: str) -> None: + """Send an email reminding the user of their username. + + Args: + email: Recipient email address. + username: The user's username to include in the message. + """ + subject = "Your DocuElevate username" + html_body = f""" + + + +
+

Your Username

+

You requested a reminder of your DocuElevate username.

+
+

Your username is:

+

{username}

+
+

You can sign in using your username or your email address.

+

If you did not request this reminder, you can safely ignore this email.

+
+

DocuElevate · Intelligent Document Processing

+
+ +""" + plain_body = ( + f"You requested a reminder of your DocuElevate username.\n\n" + f"Your username is: {username}\n\n" + "You can sign in using your username or your email address.\n\n" + "If you did not request this, please ignore this email." + ) + _smtp_send(subject, html_body, plain_body, email) + + def build_session_user(user: object) -> dict: """Build the session user dict for a LocalUser, matching the OAuth session format. diff --git a/docs/API.md b/docs/API.md index e3bca54a..809bc0a6 100644 --- a/docs/API.md +++ b/docs/API.md @@ -844,6 +844,131 @@ problem. --- +**GET** `/api/admin/users/local` + +List all local (email/password) user accounts with basic metadata. + +--- + +**POST** `/api/admin/users/local` + +Create a new local user account (admin-only, immediately active — no email verification required). + +**Request body**: +```json +{ + "email": "user@example.com", + "username": "alice", + "display_name": "Alice Smith", + "password": "securepassword", + "is_admin": false +} +``` + +--- + +**PATCH** `/api/admin/users/local/{local_user_id}` + +Update an existing local user account. Only the provided (non-null) fields are modified. +If the email is changed, the associated `UserProfile.user_id` is also updated automatically. + +**Request body** (all fields optional): +```json +{ + "email": "newemail@example.com", + "display_name": "Alice Wonderland", + "is_admin": true, + "is_active": false +} +``` + +**Error Responses**: +- `404`: Local user not found +- `409`: New email already taken by another account + +--- + +**POST** `/api/admin/users/local/{local_user_id}/send-password-reset` + +Send a password reset email to a local user on their behalf. Useful when a user is locked out. +Returns `{"sent": true}` on success or `{"sent": false, "reason": "..."}` when SMTP is not +configured or sending fails (never returns an error status so the admin always gets feedback). + +**Error Responses**: +- `404`: Local user not found + +--- + +**POST** `/api/admin/users/local/{local_user_id}/set-password` + +Directly set a new password for a local user without requiring an email token (last resort when +email delivery is unavailable). The user should be advised to change their password after logging in. + +**Request body**: +```json +{ + "password": "temporarypassword" +} +``` + +**Error Responses**: +- `404`: Local user not found +- `422`: Password shorter than 8 characters + +--- + +**DELETE** `/api/admin/users/local/{local_user_id}` + +Delete a local user account by numeric ID. The associated `UserProfile` is also removed. Documents +owned by this user are **not** deleted. Returns `204 No Content` on success. + +--- + +### Local Authentication (self-service) + +These endpoints are for local (email/password) users and do not require authentication. + +**POST** `/api/auth/request-password-reset` + +Send a password reset email. Always returns 200 to avoid leaking whether an email is registered. + +**Request body**: +```json +{ "email": "user@example.com" } +``` + +--- + +**POST** `/api/auth/reset-password` + +Set a new password using a valid reset token (received via email). + +**Request body**: +```json +{ + "token": "the-token-from-email", + "new_password": "newpassword", + "new_password_confirm": "newpassword" +} +``` + +**Error Responses**: +- `400`: Token is invalid or expired +- `422`: Passwords do not match + +--- + +**POST** `/api/auth/forgot-username` + +Send a username reminder email. Always returns 200 to avoid leaking whether an email is registered. + +**Request body**: +```json +{ "email": "user@example.com" } +``` + +--- + ### Settings Suggestions (Autocomplete) **GET** `/api/settings/{key}/suggestions` diff --git a/docs/UserGuide.md b/docs/UserGuide.md index 59ffdc40..a6332bc9 100644 --- a/docs/UserGuide.md +++ b/docs/UserGuide.md @@ -28,8 +28,29 @@ If OpenID Connect authentication is configured: 3. Log in with your existing credentials on that platform 4. You'll be redirected back to DocuElevate after successful authentication -#### User Sessions -- Once authenticated, your session will remain active until you log out or it expires +#### Local User Accounts +If your administrator has created a local (email/password) account for you: + +1. You'll see a "Sign in with username" form on the login page +2. Enter your **username or email address** — both are accepted +3. Enter your password and click **Sign in** + +##### Forgot your password? +If you can't remember your password: +1. Click **Forgot password?** below the sign-in form +2. Enter your email address and click **Send reset link** +3. Check your inbox for a password reset email (valid for 24 hours) +4. Click the link in the email and enter your new password + +##### Forgot your username? +If you can't remember your username: +1. Click **Forgot username?** below the sign-in form +2. Enter your email address and click **Send username reminder** +3. You'll receive an email with your username + +> **Tip:** You can always sign in with your email address directly — you don't need to look up your username. + + - Click the "Logout" button in the top navigation bar to end your session - For security, sessions automatically expire after a period of inactivity diff --git a/frontend/templates/admin_users.html b/frontend/templates/admin_users.html index 874c1789..360dcb2c 100644 --- a/frontend/templates/admin_users.html +++ b/frontend/templates/admin_users.html @@ -1122,7 +1122,7 @@ function adminUsersApp() { }, body: JSON.stringify({ email: this.editLocalUserModal.form.email || null, - display_name: this.editLocalUserModal.form.display_name || null, + display_name: this.editLocalUserModal.form.display_name, is_admin: this.editLocalUserModal.form.is_admin, is_active: this.editLocalUserModal.form.is_active, }), diff --git a/frontend/templates/forgot_username.html b/frontend/templates/forgot_username.html new file mode 100644 index 00000000..c8c9b8d2 --- /dev/null +++ b/frontend/templates/forgot_username.html @@ -0,0 +1,125 @@ + + + + + + DocuElevate - Forgot Username + + + + + +
+
+ DocuElevate Logo +
+ +

Forgot your username?

+

+ Enter the email address associated with your account and we'll send you your username. + You can also sign in directly with your email address. +

+ +
+
+
+
+ +
+
+

Check your inbox

+

+ If an account exists for that email address, your username has been sent. + Remember: you can also sign in using your email address directly. +

+ Back to sign in +
+ +
+ + +
+ + Tip: You can sign in with either your username or your email address — no lookup needed. +
+ +
+ + +
+ + +
+
+ + +
+
+ DocuElevate {{ app_version|default('', true) }} +
+ + diff --git a/frontend/templates/login.html b/frontend/templates/login.html index 0165035a..bcfbe815 100644 --- a/frontend/templates/login.html +++ b/frontend/templates/login.html @@ -35,8 +35,9 @@
- +
@@ -50,9 +51,13 @@ Sign in
-
+
diff --git a/tests/test_admin_users.py b/tests/test_admin_users.py index 7f215278..02cf3369 100644 --- a/tests/test_admin_users.py +++ b/tests/test_admin_users.py @@ -10,6 +10,7 @@ Covers: - Pagination and search filtering """ +from datetime import datetime, timezone from unittest.mock import MagicMock import pytest @@ -20,7 +21,7 @@ from sqlalchemy.orm import sessionmaker from sqlalchemy.pool import StaticPool from app.database import Base, get_db -from app.models import FileRecord, UserProfile +from app.models import FileRecord, LocalUser, UserProfile # --------------------------------------------------------------------------- # Fixtures @@ -663,3 +664,266 @@ class TestEnsureUserProfileAdmin: # No profile should have been created count = au_session.query(UserProfile).count() assert count == 0 + + +# --------------------------------------------------------------------------- +# Local user admin management: update, send-password-reset, set-password +# --------------------------------------------------------------------------- + + +def _make_local_user(session, email: str = "lu@example.com", username: str = "luuser", **kwargs) -> LocalUser: + """Insert a LocalUser row and return it.""" + from app.utils.local_auth import hash_password + + defaults = { + "hashed_password": hash_password("password123"), + "is_active": True, + "is_admin": False, + } + defaults.update(kwargs) + user = LocalUser(email=email, username=username, **defaults) + session.add(user) + session.commit() + session.refresh(user) + return user + + +class TestAdminUpdateLocalUser: + """Tests for PATCH /api/admin/users/local/{id}.""" + + @pytest.mark.unit + def test_update_email(self, au_client, au_session): + """PATCH can change the email address of a local user.""" + user = _make_local_user(au_session, email="old@example.com", username="updateemail") + + resp = au_client.patch( + f"/api/admin/users/local/{user.id}", + json={"email": "new@example.com"}, + ) + assert resp.status_code == 200 + assert resp.json()["email"] == "new@example.com" + + au_session.refresh(user) + assert user.email == "new@example.com" + + @pytest.mark.unit + def test_update_email_syncs_user_profile(self, au_client, au_session): + """PATCH email also updates UserProfile.user_id for the matching profile.""" + user = _make_local_user(au_session, email="synced@example.com", username="synceduser") + _make_profile(au_session, "synced@example.com") + + au_client.patch( + f"/api/admin/users/local/{user.id}", + json={"email": "synced_new@example.com"}, + ) + + from app.models import UserProfile + + old_profile = au_session.query(UserProfile).filter_by(user_id="synced@example.com").first() + new_profile = au_session.query(UserProfile).filter_by(user_id="synced_new@example.com").first() + assert old_profile is None + assert new_profile is not None + + @pytest.mark.unit + def test_update_email_conflict_returns_409(self, au_client, au_session): + """PATCH returns 409 when the new email is already taken.""" + _make_local_user(au_session, email="taken@example.com", username="takenuser") + user = _make_local_user(au_session, email="mine@example.com", username="myuser") + + resp = au_client.patch( + f"/api/admin/users/local/{user.id}", + json={"email": "taken@example.com"}, + ) + assert resp.status_code == 409 + + @pytest.mark.unit + def test_update_is_admin(self, au_client, au_session): + """PATCH can grant or revoke admin privileges.""" + user = _make_local_user(au_session, email="grantadmin@example.com", username="grantadmin") + assert user.is_admin is False + + resp = au_client.patch( + f"/api/admin/users/local/{user.id}", + json={"is_admin": True}, + ) + assert resp.status_code == 200 + assert resp.json()["is_admin"] is True + + au_session.refresh(user) + assert user.is_admin is True + + @pytest.mark.unit + def test_update_is_active(self, au_client, au_session): + """PATCH can deactivate a user account.""" + user = _make_local_user(au_session, email="deactivate@example.com", username="deactivateuser") + + resp = au_client.patch( + f"/api/admin/users/local/{user.id}", + json={"is_active": False}, + ) + assert resp.status_code == 200 + assert resp.json()["is_active"] is False + + au_session.refresh(user) + assert user.is_active is False + + @pytest.mark.unit + def test_update_display_name(self, au_client, au_session): + """PATCH can update the display name.""" + user = _make_local_user(au_session, email="displayname@example.com", username="displaynameuser") + + resp = au_client.patch( + f"/api/admin/users/local/{user.id}", + json={"display_name": "Alice Wonderland"}, + ) + assert resp.status_code == 200 + assert resp.json()["display_name"] == "Alice Wonderland" + + @pytest.mark.unit + def test_update_nonexistent_user_returns_404(self, au_client): + """PATCH on unknown ID returns 404.""" + resp = au_client.patch("/api/admin/users/local/99999", json={"email": "x@example.com"}) + assert resp.status_code == 404 + + +class TestAdminSendPasswordReset: + """Tests for POST /api/admin/users/local/{id}/send-password-reset.""" + + @pytest.mark.unit + def test_send_reset_email_success(self, au_client, au_session): + """Returns sent=True when SMTP is configured and sending succeeds.""" + from unittest.mock import patch + + user = _make_local_user(au_session, email="resetme@example.com", username="resetmeuser") + + with ( + patch("app.api.admin_users.settings") as mock_settings, + patch("app.api.admin_users.send_password_reset_email") as mock_send, + ): + mock_settings.email_host = "smtp.example.com" + mock_settings.version = "test" + resp = au_client.post(f"/api/admin/users/local/{user.id}/send-password-reset") + + assert resp.status_code == 200 + data = resp.json() + assert data["sent"] is True + assert data["email"] == "resetme@example.com" + mock_send.assert_called_once() + + @pytest.mark.unit + def test_send_reset_email_no_smtp_returns_not_sent(self, au_client, au_session): + """Returns sent=False with reason when SMTP is not configured.""" + from unittest.mock import patch + + user = _make_local_user(au_session, email="nosmtp@example.com", username="nosmtpuser") + + with patch("app.api.admin_users.settings") as mock_settings: + mock_settings.email_host = "" + resp = au_client.post(f"/api/admin/users/local/{user.id}/send-password-reset") + + assert resp.status_code == 200 + data = resp.json() + assert data["sent"] is False + assert "smtp" in data["reason"].lower() + + @pytest.mark.unit + def test_send_reset_email_smtp_failure_returns_not_sent(self, au_client, au_session): + """Returns sent=False with reason when SMTP sending fails.""" + from unittest.mock import patch + + user = _make_local_user(au_session, email="smtperr@example.com", username="smtperruser") + + with ( + patch("app.api.admin_users.settings") as mock_settings, + patch("app.api.admin_users.send_password_reset_email", side_effect=RuntimeError("connection refused")), + ): + mock_settings.email_host = "smtp.example.com" + resp = au_client.post(f"/api/admin/users/local/{user.id}/send-password-reset") + + assert resp.status_code == 200 + assert resp.json()["sent"] is False + + @pytest.mark.unit + def test_send_reset_email_unknown_user_returns_404(self, au_client): + """Returns 404 for unknown local_user_id.""" + resp = au_client.post("/api/admin/users/local/99999/send-password-reset") + assert resp.status_code == 404 + + @pytest.mark.unit + def test_send_reset_stores_token(self, au_client, au_session): + """Password reset token is persisted to the DB.""" + from unittest.mock import patch + + user = _make_local_user(au_session, email="tokenstore@example.com", username="tokenstoreuser") + assert user.password_reset_token is None + + with ( + patch("app.api.admin_users.settings") as mock_settings, + patch("app.api.admin_users.send_password_reset_email"), + ): + mock_settings.email_host = "smtp.example.com" + au_client.post(f"/api/admin/users/local/{user.id}/send-password-reset") + + au_session.refresh(user) + assert user.password_reset_token is not None + assert user.password_reset_sent_at is not None + + +class TestAdminSetPassword: + """Tests for POST /api/admin/users/local/{id}/set-password.""" + + @pytest.mark.unit + def test_set_password_success(self, au_client, au_session): + """Returns updated=True and changes the hashed password.""" + from app.utils.local_auth import verify_password + + user = _make_local_user(au_session, email="setpw@example.com", username="setpwuser") + + resp = au_client.post( + f"/api/admin/users/local/{user.id}/set-password", + json={"password": "brandnewpassword"}, + ) + assert resp.status_code == 200 + assert resp.json()["updated"] is True + + au_session.refresh(user) + assert verify_password("brandnewpassword", user.hashed_password) + + @pytest.mark.unit + def test_set_password_too_short_returns_422(self, au_client, au_session): + """Returns 422 when password is shorter than 8 characters.""" + user = _make_local_user(au_session, email="shortpw@example.com", username="shortpwuser") + + resp = au_client.post( + f"/api/admin/users/local/{user.id}/set-password", + json={"password": "short"}, + ) + assert resp.status_code == 422 + + @pytest.mark.unit + def test_set_password_clears_reset_token(self, au_client, au_session): + """Setting a password clears any outstanding password_reset_token.""" + from app.utils.local_auth import generate_token + + user = _make_local_user(au_session, email="cleartok@example.com", username="cleartokuser") + user.password_reset_token = generate_token() + user.password_reset_sent_at = datetime.now(tz=timezone.utc) + au_session.commit() + + au_client.post( + f"/api/admin/users/local/{user.id}/set-password", + json={"password": "clearedpassword"}, + ) + + au_session.refresh(user) + assert user.password_reset_token is None + assert user.password_reset_sent_at is None + + @pytest.mark.unit + def test_set_password_unknown_user_returns_404(self, au_client): + """Returns 404 for unknown local_user_id.""" + resp = au_client.post( + "/api/admin/users/local/99999/set-password", + json={"password": "doesnotmatter"}, + ) + assert resp.status_code == 404 diff --git a/tests/test_local_auth.py b/tests/test_local_auth.py index c6a11c98..52cb9708 100644 --- a/tests/test_local_auth.py +++ b/tests/test_local_auth.py @@ -6,9 +6,12 @@ Covers: - POST /api/auth/resend-verification - POST /api/auth/request-password-reset - POST /api/auth/reset-password +- POST /api/auth/forgot-username - GET /signup (page route) - GET /verify-email-sent (page route) - GET /reset-password (page route) +- GET /forgot-password (page route) +- GET /forgot-username (page route) - app/utils/local_auth utility functions - auth() login flow with LocalUser """ @@ -801,3 +804,116 @@ def test_admin_local_user_list_after_create(admin_session_client): assert resp.status_code == 200 users = resp.json() assert any(u["email"] == "listed@example.com" for u in users) + + +# --------------------------------------------------------------------------- +# Integration tests: forgot-username endpoint +# --------------------------------------------------------------------------- + + +@pytest.mark.integration +def test_forgot_username_returns_200_for_existing_email(la_client, la_session): + """POST /api/auth/forgot-username returns 200 and sends email when account exists.""" + la_session.add( + LocalUser( + email="remindme@example.com", + username="remindmeuser", + hashed_password=hash_password("pw123456"), + is_active=True, + ) + ) + la_session.commit() + + with patch("app.api.local_auth.send_forgot_username_email") as mock_send: + resp = la_client.post("/api/auth/forgot-username", json={"email": "remindme@example.com"}) + + assert resp.status_code == 200 + assert "reminder" in resp.json()["message"].lower() + mock_send.assert_called_once_with("remindme@example.com", "remindmeuser") + + +@pytest.mark.integration +def test_forgot_username_returns_200_for_unknown_email(la_client): + """POST /api/auth/forgot-username always returns 200 (no info leak).""" + with patch("app.api.local_auth.send_forgot_username_email") as mock_send: + resp = la_client.post("/api/auth/forgot-username", json={"email": "nobody@example.com"}) + + assert resp.status_code == 200 + mock_send.assert_not_called() + + +@pytest.mark.integration +def test_forgot_username_smtp_failure_does_not_raise(la_client, la_session): + """POST /api/auth/forgot-username returns 200 even when SMTP fails.""" + la_session.add( + LocalUser( + email="smtpfail@example.com", + username="smtpfailuser", + hashed_password=hash_password("pw123456"), + is_active=True, + ) + ) + la_session.commit() + + with patch("app.api.local_auth.send_forgot_username_email", side_effect=RuntimeError("SMTP down")): + resp = la_client.post("/api/auth/forgot-username", json={"email": "smtpfail@example.com"}) + + assert resp.status_code == 200 + + +# --------------------------------------------------------------------------- +# Integration tests: new page routes +# --------------------------------------------------------------------------- + + +@pytest.mark.integration +def test_forgot_password_page(la_client): + """GET /forgot-password returns 200.""" + with patch("app.api.local_auth.settings") as mock_settings: + mock_settings.version = "test" + resp = la_client.get("/forgot-password") + assert resp.status_code == 200 + assert b"password" in resp.content.lower() + + +@pytest.mark.integration +def test_forgot_username_page(la_client): + """GET /forgot-username returns 200.""" + with patch("app.api.local_auth.settings") as mock_settings: + mock_settings.version = "test" + resp = la_client.get("/forgot-username") + assert resp.status_code == 200 + assert b"username" in resp.content.lower() + + +# --------------------------------------------------------------------------- +# Unit tests: send_forgot_username_email utility +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +def test_send_forgot_username_email_calls_smtp(): + """send_forgot_username_email calls _smtp_send with the username.""" + from app.utils.local_auth import send_forgot_username_email + + with patch("app.utils.local_auth._smtp_send") as mock_smtp: + send_forgot_username_email("u@example.com", "myusername") + + mock_smtp.assert_called_once() + args = mock_smtp.call_args[0] + # subject, html_body, plain_body, recipient + assert "myusername" in args[1] # HTML body + assert "myusername" in args[2] # plain body + assert args[3] == "u@example.com" + + +@pytest.mark.unit +def test_send_forgot_username_email_no_smtp_raises(): + """send_forgot_username_email raises RuntimeError when EMAIL_HOST is not set.""" + from app.utils.local_auth import send_forgot_username_email + + with patch("app.utils.local_auth.settings") as mock_settings: + mock_settings.email_host = "" + + with pytest.raises(RuntimeError, match="SMTP"): + send_forgot_username_email("u@example.com", "myusername") From 58c9b5d7f01941949db71b2816678ac6464d9d0f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Mar 2026 10:10:18 +0000 Subject: [PATCH 10/25] fix(email): create missing email template and decouple email destination settings - Create app/templates/email/default.html (fixes 'default.html not found' error) - Add DEST_EMAIL_* settings to app/config.py (decoupled from shared EMAIL_* settings) - Update upload_to_email task to use dest_email_* settings exclusively - Update _should_upload_to_email() to check dest_email_* settings - Update config validator, providers, and settings_service for dest_email_* - Update .env.demo and docs/ConfigurationGuide.md - Update all tests to use dest_email_* settings where appropriate" Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> --- .env.demo | 16 +++++- app/config.py | 11 ++++- 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_config_validators.py | 14 +++--- tests/test_coverage_remaining_gaps.py | 7 +++ tests/test_send_to_all.py | 8 +-- tests/test_upload_email.py | 50 +++++++++---------- tests/test_upload_tasks.py | 14 +++--- 14 files changed, 260 insertions(+), 82 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/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/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 %} + + {% endif %} + +

+ This document was sent automatically by {{ app_name }}.{% if app_url %} Visit {{ app_url }} to manage your documents.{% 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_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_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" From ff1310c23ed6080f95957bf19fdb7cc179d10ecb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Mar 2026 10:10:46 +0000 Subject: [PATCH 11/25] fix(tasks): remove erroneous in_progress log that regressed finalize_document_storage status when PDF/A archival is enabled Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> --- app/tasks/finalize_document_storage.py | 7 --- tests/test_finalize_storage.py | 66 ++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 7 deletions(-) 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/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}" + ) From 52b3f153291ee4a5519d519c755c3ee1dd44e95e Mon Sep 17 00:00:00 2001 From: semantic-release Date: Sun, 8 Mar 2026 10:53:10 +0000 Subject: [PATCH 12/25] 0.90.1 Automatically generated by python-semantic-release --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 14e7b27d..3a7a4e8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 +## 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 From 70e188125b1644485bae9884d99c5f41772bda2f Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Sun, 8 Mar 2026 10:53:13 +0000 Subject: [PATCH 13/25] chore(release): update build metadata files [skip ci] --- BUILD_DATE | 2 +- GIT_SHA | 2 +- RUNTIME_INFO | 12 ++++++------ VERSION | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/BUILD_DATE b/BUILD_DATE index fc8a0076..282c9eff 100644 --- a/BUILD_DATE +++ b/BUILD_DATE @@ -1 +1 @@ -2026-03-08T09:05:40Z +2026-03-08T10:53:10Z diff --git a/GIT_SHA b/GIT_SHA index df92da07..112b6081 100644 --- a/GIT_SHA +++ b/GIT_SHA @@ -1 +1 @@ -bd9da65 +7e2d392 diff --git a/RUNTIME_INFO b/RUNTIME_INFO index 6eba300d..4f9536a0 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.1 +Build Date: 2026-03-08T10:53:10Z +Git Commit: 7e2d392791796dd9ce043c77d6c6ad876f7a770a +Git Short SHA: 7e2d392 Git Branch: main -Commit Date: 2026-03-08T10:05:10+01:00 -Build Timestamp: 2026-03-08T09:05:40Z +Commit Date: 2026-03-08T11:52:16+01:00 +Build Timestamp: 2026-03-08T10:53:10Z ============================== diff --git a/VERSION b/VERSION index ae02209b..fa3fb18d 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.90.0 +0.90.1 From 4de739c5664911cf2aa1aace050fc3cbfee45784 Mon Sep 17 00:00:00 2001 From: semantic-release Date: Sun, 8 Mar 2026 10:58:32 +0000 Subject: [PATCH 14/25] 0.90.2 Automatically generated by python-semantic-release --- CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a7a4e8e..3aa045d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 +## 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 From 42dc335747372e88102790d7cd034ccffd74c94a Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Sun, 8 Mar 2026 10:58:35 +0000 Subject: [PATCH 15/25] chore(release): update build metadata files [skip ci] --- BUILD_DATE | 2 +- GIT_SHA | 2 +- RUNTIME_INFO | 12 ++++++------ VERSION | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/BUILD_DATE b/BUILD_DATE index 282c9eff..aea5d546 100644 --- a/BUILD_DATE +++ b/BUILD_DATE @@ -1 +1 @@ -2026-03-08T10:53:10Z +2026-03-08T10:58:32Z diff --git a/GIT_SHA b/GIT_SHA index 112b6081..7db6b9ae 100644 --- a/GIT_SHA +++ b/GIT_SHA @@ -1 +1 @@ -7e2d392 +a321858 diff --git a/RUNTIME_INFO b/RUNTIME_INFO index 4f9536a0..9111069b 100644 --- a/RUNTIME_INFO +++ b/RUNTIME_INFO @@ -1,10 +1,10 @@ DocuElevate Build Information ============================== -Version: 0.90.1 -Build Date: 2026-03-08T10:53:10Z -Git Commit: 7e2d392791796dd9ce043c77d6c6ad876f7a770a -Git Short SHA: 7e2d392 +Version: 0.90.2 +Build Date: 2026-03-08T10:58:32Z +Git Commit: a3218583c6b2de96968dbd30d46a03c146ab2082 +Git Short SHA: a321858 Git Branch: main -Commit Date: 2026-03-08T11:52:16+01:00 -Build Timestamp: 2026-03-08T10:53:10Z +Commit Date: 2026-03-08T11:58:14+01:00 +Build Timestamp: 2026-03-08T10:58:32Z ============================== diff --git a/VERSION b/VERSION index fa3fb18d..654fae02 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.90.1 +0.90.2 From b7bf4f352d1dd305f379020108e3674d3c25b5c5 Mon Sep 17 00:00:00 2001 From: semantic-release Date: Sun, 8 Mar 2026 10:58:56 +0000 Subject: [PATCH 16/25] 0.90.3 Automatically generated by python-semantic-release --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3aa045d0..6778a21b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,14 @@ 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 From 97616beb117cf461697cd62377a1c5e454d5b446 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Sun, 8 Mar 2026 10:58:59 +0000 Subject: [PATCH 17/25] chore(release): update build metadata files [skip ci] --- BUILD_DATE | 2 +- GIT_SHA | 2 +- RUNTIME_INFO | 12 ++++++------ VERSION | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/BUILD_DATE b/BUILD_DATE index aea5d546..98bb6ac5 100644 --- a/BUILD_DATE +++ b/BUILD_DATE @@ -1 +1 @@ -2026-03-08T10:58:32Z +2026-03-08T10:58:56Z diff --git a/GIT_SHA b/GIT_SHA index 7db6b9ae..ac40d760 100644 --- a/GIT_SHA +++ b/GIT_SHA @@ -1 +1 @@ -a321858 +da47283 diff --git a/RUNTIME_INFO b/RUNTIME_INFO index 9111069b..2a136b0a 100644 --- a/RUNTIME_INFO +++ b/RUNTIME_INFO @@ -1,10 +1,10 @@ DocuElevate Build Information ============================== -Version: 0.90.2 -Build Date: 2026-03-08T10:58:32Z -Git Commit: a3218583c6b2de96968dbd30d46a03c146ab2082 -Git Short SHA: a321858 +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-08T11:58:14+01:00 -Build Timestamp: 2026-03-08T10:58:32Z +Commit Date: 2026-03-08T11:58:40+01:00 +Build Timestamp: 2026-03-08T10:58:56Z ============================== diff --git a/VERSION b/VERSION index 654fae02..c7709f43 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.90.2 +0.90.3 From 6c07d70d93f6f94717258a0a137bd90ec3d35d1c Mon Sep 17 00:00:00 2001 From: semantic-release Date: Sun, 8 Mar 2026 11:02:37 +0000 Subject: [PATCH 18/25] 0.91.0 Automatically generated by python-semantic-release --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6778a21b..b8c26204 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 +## v0.91.0 (2026-03-08) + +### Features + +- **backup**: Extend backup and restore to PostgreSQL and MySQL/MariaDB + ([`a0f5ba1`](https://github.com/christianlouis/DocuElevate/commit/a0f5ba179978d7c63bbdb8c564a32eaf3f78e5ad)) + + ## v0.90.3 (2026-03-08) ### Bug Fixes From d5af0903e2440816b0b1a6c107dd6359dcb97bdb Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Sun, 8 Mar 2026 11:02:39 +0000 Subject: [PATCH 19/25] chore(release): update build metadata files [skip ci] --- BUILD_DATE | 2 +- GIT_SHA | 2 +- RUNTIME_INFO | 12 ++++++------ VERSION | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/BUILD_DATE b/BUILD_DATE index 98bb6ac5..d09f6c05 100644 --- a/BUILD_DATE +++ b/BUILD_DATE @@ -1 +1 @@ -2026-03-08T10:58:56Z +2026-03-08T11:02:37Z diff --git a/GIT_SHA b/GIT_SHA index ac40d760..b666792d 100644 --- a/GIT_SHA +++ b/GIT_SHA @@ -1 +1 @@ -da47283 +db40e85 diff --git a/RUNTIME_INFO b/RUNTIME_INFO index 2a136b0a..893747ae 100644 --- a/RUNTIME_INFO +++ b/RUNTIME_INFO @@ -1,10 +1,10 @@ DocuElevate Build Information ============================== -Version: 0.90.3 -Build Date: 2026-03-08T10:58:56Z -Git Commit: da47283e0afb48e2992c7698b5ff74028c83d55e -Git Short SHA: da47283 +Version: 0.91.0 +Build Date: 2026-03-08T11:02:37Z +Git Commit: db40e85856d59f73a49111282f94d34810bb24d5 +Git Short SHA: db40e85 Git Branch: main -Commit Date: 2026-03-08T11:58:40+01:00 -Build Timestamp: 2026-03-08T10:58:56Z +Commit Date: 2026-03-08T12:02:17+01:00 +Build Timestamp: 2026-03-08T11:02:37Z ============================== diff --git a/VERSION b/VERSION index c7709f43..8f63f4f9 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.90.3 +0.91.0 From 9b8eb911c3bf2b0be84a73030e82cff12151d4ba Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Mar 2026 11:07:37 +0000 Subject: [PATCH 20/25] fix(admin-users): restore missing @router.get decorator on get_user endpoint The @router.get("/{user_id:path}") decorator was accidentally dropped from the get_user function when the /local/... routes were inserted above it in the previous PR. Without the decorator the function was never registered as a GET handler, so GET /api/admin/users/ matched the PUT/DELETE catch-all routes and Starlette correctly returned 405 Method Not Allowed instead of 200/403. Adding the decorator back restores the GET endpoint and fixes the 5 tests that were failing with 405. Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> --- app/api/admin_users.py | 1 + 1 file changed, 1 insertion(+) diff --git a/app/api/admin_users.py b/app/api/admin_users.py index d73a1467..67d453ef 100644 --- a/app/api/admin_users.py +++ b/app/api/admin_users.py @@ -502,6 +502,7 @@ def admin_set_password( return {"updated": True, "email": user.email} +@router.get("/{user_id:path}", summary="Get details for a single user") def get_user(user_id: str, db: DbSession, _admin: AdminUser) -> dict[str, Any]: """Return profile and document statistics for a specific user.""" doc_count = db.query(func.count(FileRecord.id)).filter(FileRecord.owner_id == user_id).scalar() or 0 From 962ee09f306426c8339fc53b19b44fdb9b344c8c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Mar 2026 11:10:14 +0000 Subject: [PATCH 21/25] Initial plan From 3f8a95d8081c4ce3ad380e6391afa3bebb57bbb7 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:34 +0000 Subject: [PATCH 22/25] ci: fix CodeQL javascript language identifier mismatch Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> --- .github/workflows/codeql.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 77a4caae..5c808b2a 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -24,7 +24,7 @@ jobs: include: - language: actions build-mode: none - - language: javascript-typescript + - language: javascript build-mode: none - language: python build-mode: none From f2d2aaa3545081de1ff7317672199085daa3c3c0 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Sun, 8 Mar 2026 11:13:43 +0000 Subject: [PATCH 23/25] docs(changelog): update changelog [skip ci] --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b8c26204..0dacc034 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 +## Unreleased + +### Continuous Integration + +- Fix CodeQL javascript language identifier mismatch + ([`3f8a95d`](https://github.com/christianlouis/DocuElevate/commit/3f8a95d8081c4ce3ad380e6391afa3bebb57bbb7)) + + ## v0.91.0 (2026-03-08) ### Features From 0f21479abc1d87d7f31e64fcd06f940002cfcd4e Mon Sep 17 00:00:00 2001 From: semantic-release Date: Sun, 8 Mar 2026 11:14:23 +0000 Subject: [PATCH 24/25] 0.92.0 Automatically generated by python-semantic-release --- CHANGELOG.md | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0dacc034..ef663762 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,34 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 +## v0.92.0 (2026-03-08) + +### Bug Fixes + +- **admin-users**: Restore missing @router.get decorator on get_user endpoint + ([`9b8eb91`](https://github.com/christianlouis/DocuElevate/commit/9b8eb911c3bf2b0be84a73030e82cff12151d4ba)) + +### Chores + +- Update plan to include forgot-username and login label clarification + ([`44ea43f`](https://github.com/christianlouis/DocuElevate/commit/44ea43f9cf948aabe8f8ae98744cd41061ab8770)) + +### Continuous Integration + +- Fix CodeQL javascript language identifier mismatch + ([`3f8a95d`](https://github.com/christianlouis/DocuElevate/commit/3f8a95d8081c4ce3ad380e6391afa3bebb57bbb7)) + +### Documentation + +- **changelog**: Update changelog [skip ci] + ([`f2d2aaa`](https://github.com/christianlouis/DocuElevate/commit/f2d2aaa3545081de1ff7317672199085daa3c3c0)) + +### Features + +- **auth**: Password reset, forgot username, and admin user management for local accounts + ([`d36ba88`](https://github.com/christianlouis/DocuElevate/commit/d36ba88de765b688888c6e661256f8508da86d89)) + + ## Unreleased ### Continuous Integration From f03227c248ad4a4230f6bf82b874d354dc042cc8 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Sun, 8 Mar 2026 11:14:26 +0000 Subject: [PATCH 25/25] chore(release): update build metadata files [skip ci] --- BUILD_DATE | 2 +- GIT_SHA | 2 +- RUNTIME_INFO | 12 ++++++------ VERSION | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/BUILD_DATE b/BUILD_DATE index d09f6c05..6c2a1b0d 100644 --- a/BUILD_DATE +++ b/BUILD_DATE @@ -1 +1 @@ -2026-03-08T11:02:37Z +2026-03-08T11:14:23Z diff --git a/GIT_SHA b/GIT_SHA index b666792d..c8c67d54 100644 --- a/GIT_SHA +++ b/GIT_SHA @@ -1 +1 @@ -db40e85 +4109bf6 diff --git a/RUNTIME_INFO b/RUNTIME_INFO index 893747ae..b1fec892 100644 --- a/RUNTIME_INFO +++ b/RUNTIME_INFO @@ -1,10 +1,10 @@ DocuElevate Build Information ============================== -Version: 0.91.0 -Build Date: 2026-03-08T11:02:37Z -Git Commit: db40e85856d59f73a49111282f94d34810bb24d5 -Git Short SHA: db40e85 +Version: 0.92.0 +Build Date: 2026-03-08T11:14:23Z +Git Commit: 4109bf65d2e127b9fa0186a6f22b5bcb685580ea +Git Short SHA: 4109bf6 Git Branch: main -Commit Date: 2026-03-08T12:02:17+01:00 -Build Timestamp: 2026-03-08T11:02:37Z +Commit Date: 2026-03-08T12:14:04+01:00 +Build Timestamp: 2026-03-08T11:14:23Z ============================== diff --git a/VERSION b/VERSION index 8f63f4f9..36545ad3 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.91.0 +0.92.0