Merge pull request #767 from christianlouis/copilot/fix-authentication-configuration-issues

fix(upload): reject exact duplicates at upload time; prevent duplicate mobile share uploads

app/api/files.py

@@ -1268,7 +1268,12 @@ async def _save_upload_file_chunks(file: UploadFile, target_path: str, max_size:
 
 
 def _check_for_exact_duplicate(db: DbSession, target_path: str, safe_filename: str) -> dict | None:
-    """Check for an exact duplicate of the uploaded file and return a warning if found."""
+    """Check for an exact duplicate of the uploaded file.
+
+    Returns a dict with duplicate info when the file's SHA-256 hash matches an
+    already-processed document, or ``None`` when no duplicate is found (or
+    deduplication is disabled).
+    """
     if not settings.enable_deduplication:
         return None
 
@@ -1287,8 +1292,8 @@ def _check_for_exact_duplicate(db: DbSession, target_path: str, safe_filename: s
                 "original_file_id": existing.id,
                 "original_filename": existing.original_filename,
                 "message": (
-                    "This file appears to be an exact duplicate of an already-processed document. "
-                    "It will still be queued but will be flagged as a duplicate."
+                    "This file is an exact duplicate of an already-processed document. "
+                    "It has not been queued for processing again."
                 ),
             }
     except Exception as e:
@@ -1390,6 +1395,25 @@ async def ui_upload(
     logger.info(f"Saved uploaded file '{safe_filename}' as '{target_filename}'")
     file_size = written_size
 
+    # ── Early duplicate rejection ──────────────────────────────────────────
+    # Check for exact duplicates (same SHA-256 hash) BEFORE enqueuing a
+    # processing task.  When deduplication is enabled and the file already
+    # exists, we skip processing entirely, clean up the temp file, and
+    # return the existing file's information to the caller.
+    exact_duplicate = _check_for_exact_duplicate(db, target_path, safe_filename)
+    if exact_duplicate:
+        # Remove the just-saved temp file — it's a duplicate.
+        try:
+            os.remove(target_path)
+        except OSError:
+            pass
+        return {
+            "status": "duplicate",
+            "original_filename": safe_filename,
+            "stored_filename": target_filename,
+            "duplicate_of": exact_duplicate,
+        }
+
     # Determine if the file is a PDF or needs conversion
     mime_type, _ = mimetypes.guess_type(target_path)
     file_ext = os.path.splitext(target_path)[1].lower()
@@ -1466,20 +1490,12 @@ async def ui_upload(
         logger.warning(f"Unsupported MIME type {mime_type} for {target_path}, attempting conversion")
         task = convert_to_pdf.delay(target_path, original_filename=safe_filename, owner_id=upload_owner_id)
 
-    # Check for exact duplicates (same SHA-256 hash) before returning.
-    # This gives the caller an immediate warning without waiting for the pipeline.
-    # Only performed when deduplication is enabled in settings.
-    exact_duplicate_warning = _check_for_exact_duplicate(db, target_path, safe_filename)
-
-    response: dict = {
+    return {
         "task_id": task.id,
         "status": "queued",
         "original_filename": safe_filename,
         "stored_filename": target_filename,
     }
-    if exact_duplicate_warning:
-        response["duplicate_warning"] = exact_duplicate_warning
-    return response
 
 
 # ---------------------------------------------------------------------------

docs/API.md

@@ -242,17 +242,33 @@ The DocuElevate browser extension uses this endpoint to send files directly from
 
 **POST** `/api/ui-upload`
 
-Upload one or more files from your computer for processing.
+Upload a file from your computer for processing.
 
 **Request**:
-- Multipart form data with file(s)
+- Multipart form data with a single `file` field
 
-**Response**:
+**Response** (new file):
 ```json
 {
-  "success": true,
-  "file_ids": [123, 124],
-  "message": "Files uploaded and queued for processing"
+  "task_id": "abc-123",
+  "status": "queued",
+  "original_filename": "invoice.pdf",
+  "stored_filename": "a1b2c3d4.pdf"
+}
+```
+
+**Response** (exact duplicate, when `ENABLE_DEDUPLICATION=True`):
+```json
+{
+  "status": "duplicate",
+  "original_filename": "invoice.pdf",
+  "stored_filename": "e5f6a7b8.pdf",
+  "duplicate_of": {
+    "duplicate_type": "exact",
+    "original_file_id": 42,
+    "original_filename": "invoice.pdf",
+    "message": "This file is an exact duplicate of an already-processed document. It has not been queued for processing again."
+  }
 }
 ```
 

docs/ConfigurationGuide.md

@@ -1581,14 +1581,30 @@ DocuElevate detects and flags documents that share the same content, even if the
 
 ### Exact Duplicate Detection (SHA-256)
 
-When `ENABLE_DEDUPLICATION=True` (the default), each new document is hashed with SHA-256 before processing begins. If the hash matches an existing file record the new document is stored as a duplicate (`is_duplicate=True`, `duplicate_of_id=<original_id>`) and no further processing is performed.
+When `ENABLE_DEDUPLICATION=True` (the default), each new document is hashed with SHA-256 before processing begins. If the hash matches an existing file record the upload is rejected immediately — no processing task is created, and the temporary file is removed from disk. The `/api/ui-upload` response returns `"status": "duplicate"` together with a `duplicate_of` object that identifies the original file.
+
+If the same file somehow reaches the Celery worker (e.g. via a watch-folder ingest) it is still caught there and stored as a duplicate (`is_duplicate=True`, `duplicate_of_id=<original_id>`) with no further processing.
 
 | Variable | Description | Default |
 |---|---|---|
 | `ENABLE_DEDUPLICATION` | Hash-based exact duplicate detection on ingest. | `True` |
 | `SHOW_DEDUPLICATION_STEP` | Show the "Check for Duplicates" step in the processing timeline UI. | `True` |
 
-An immediate duplicate warning is also included in the `/api/ui-upload` JSON response so the frontend can alert the user before the pipeline completes.
+When the upload is an exact duplicate the `/api/ui-upload` response looks like:
+
+```json
+{
+  "status": "duplicate",
+  "original_filename": "invoice.pdf",
+  "stored_filename": "abc-123.pdf",
+  "duplicate_of": {
+    "duplicate_type": "exact",
+    "original_file_id": 42,
+    "original_filename": "invoice.pdf",
+    "message": "This file is an exact duplicate of an already-processed document. It has not been queued for processing again."
+  }
+}
+```
 
 ### Near-Duplicate Detection (Content Similarity)
 

frontend/static/js/upload.js

@@ -433,9 +433,17 @@ function _uploadSingleFile(file, progressBar, statusEl, onTerminal) {
       if (xhr.status === 200) {
         const result = JSON.parse(xhr.responseText);
         progressBar.style.width = '100%';
-        progressBar.className = 'file-progress-bar bg-green-500 h-2 rounded-full';
-        statusEl.textContent = `Success: Task ID: ${result.task_id}`;
-        statusEl.className = 'text-xs text-green-600 mt-1';
+
+        if (result.status === 'duplicate' && result.duplicate_of) {
+          // Exact duplicate – no processing task was created
+          progressBar.className = 'file-progress-bar bg-yellow-400 h-2 rounded-full';
+          statusEl.textContent = `Duplicate – already processed (file #${result.duplicate_of.original_file_id})`;
+          statusEl.className = 'text-xs text-yellow-600 mt-1';
+        } else {
+          progressBar.className = 'file-progress-bar bg-green-500 h-2 rounded-full';
+          statusEl.textContent = `Success: Task ID: ${result.task_id}`;
+          statusEl.className = 'text-xs text-green-600 mt-1';
+        }
         _onUploadSuccess();
         onTerminal();
         resolve({ rateLimited: false, retryAfterSeconds: 0 });

mobile/app/+not-found.tsx

@@ -110,7 +110,14 @@ export default function NotFoundScreen() {
   const router = useRouter();
   const { addPendingFile } = useShare();
 
+  // Guard: track which pathname has been handled so the effect does not
+  // re-fire when `router` or `addPendingFile` change identity mid-navigation.
+  const handledRef = React.useRef<string | null>(null);
+
   useEffect(() => {
+    if (handledRef.current === pathname) return;  // already handled
+    handledRef.current = pathname;
+
     if (looksLikeFilePath(pathname)) {
       // Filesystem path from iOS "Open In…" – add the file to ShareContext
       // and redirect to the Upload tab.  UploadScreen will pick up the

mobile/src/context/ShareContext.tsx

@@ -9,6 +9,7 @@
  */
 
 import React, { createContext, useCallback, useContext, useState } from "react";
+import { normalizeFileUri } from "../utils/normalizeUri";
 
 export interface SharedFile {
   uri: string;
@@ -35,11 +36,8 @@ export function ShareProvider({ children }: { children: React.ReactNode }) {
     setPendingFiles((prev) => {
       // Deduplicate by normalised URI so the same file is not uploaded twice
       // when both the Linking handler (_layout.tsx) and +not-found.tsx fire.
-      const normalize = (uri: string) => {
-        try { return decodeURIComponent(uri); } catch { return uri; }
-      };
-      const norm = normalize(file.uri);
-      if (prev.some((f) => normalize(f.uri) === norm)) return prev;
+      const norm = normalizeFileUri(file.uri);
+      if (prev.some((f) => normalizeFileUri(f.uri) === norm)) return prev;
       return [...prev, file];
     });
   }, []);

mobile/src/screens/UploadScreen.tsx

@@ -28,6 +28,7 @@ import {
 } from "react-native";
 import { useAuth } from "../context/AuthContext";
 import { useShare } from "../context/ShareContext";
+import { normalizeFileUri } from "../utils/normalizeUri";
 import api from "../services/api";
 
 /** Statuses that indicate processing has finished (no further polling needed). */
@@ -63,6 +64,11 @@ export default function UploadScreen() {
     uploadsRef.current = uploads;
   }, [uploads]);
 
+  // Track URIs that have already been uploaded in this session so that
+  // duplicate share-sheet deliveries (iOS can fire both the Linking handler
+  // and +not-found.tsx for the same file) do not trigger repeated uploads.
+  const uploadedUrisRef = useRef<Set<string>>(new Set());
+
   // ---------------------------------------------------------------------------
   // Core helpers (declared before the effects that depend on them)
   // ---------------------------------------------------------------------------
@@ -103,20 +109,51 @@ export default function UploadScreen() {
   }, []);
 
   const uploadFile = useCallback(async (uri: string, filename: string, mimeType?: string) => {
-    const id = `${Date.now()}-${filename}`;
+    // Deduplicate: skip if this exact URI was already uploaded in this session.
+    // This guards against duplicate share-sheet deliveries from iOS where the
+    // Linking handler and +not-found.tsx fire for the same file.
+    const normUri = normalizeFileUri(uri);
+    if (uploadedUrisRef.current.has(normUri)) {
+      console.debug("[uploadFile] skipping duplicate URI:", uri);
+      return;
+    }
+    uploadedUrisRef.current.add(normUri);
+
+    const id = `${Date.now()}-${Math.random().toString(36).slice(2, 6)}-${filename}`;
     setUploads((prev) => [{ id, filename, status: "uploading", uri, mimeType }, ...prev]);
 
     try {
       const localUri = await ensureLocalUri(uri, filename);
       const resp = await api.uploadFile(localUri, filename, mimeType);
-      setUploads((prev) =>
-        prev.map((item) =>
-          item.id === id
-            ? { ...item, status: "done", taskId: resp.task_id, originalFilename: resp.original_filename }
-            : item
-        )
-      );
+      if (resp.status === "duplicate" && resp.duplicate_of) {
+        // Server rejected the file as a known duplicate — mark as done and
+        // set the server-side status to "duplicate" so it appears as a
+        // terminal status and is not polled further.
+        setUploads((prev) =>
+          prev.map((item) =>
+            item.id === id
+              ? {
+                  ...item,
+                  status: "done",
+                  fileId: resp.duplicate_of!.original_file_id,
+                  originalFilename: resp.original_filename,
+                  serverStatus: "duplicate",
+                }
+              : item
+          )
+        );
+      } else {
+        setUploads((prev) =>
+          prev.map((item) =>
+            item.id === id
+              ? { ...item, status: "done", taskId: resp.task_id, originalFilename: resp.original_filename }
+              : item
+          )
+        );
+      }
     } catch (err: unknown) {
+      // Allow retrying this URI on failure.
+      uploadedUrisRef.current.delete(normUri);
       const msg = err instanceof Error ? err.message : "Upload failed";
       setUploads((prev) =>
         prev.map((item) => (item.id === id ? { ...item, status: "error", error: msg } : item))
@@ -139,13 +176,29 @@ export default function UploadScreen() {
     try {
       const localUri = await ensureLocalUri(item.uri, item.filename);
       const resp = await api.uploadFile(localUri, item.filename, item.mimeType);
-      setUploads((prev) =>
-        prev.map((u) =>
-          u.id === item.id
-            ? { ...u, status: "done", taskId: resp.task_id, originalFilename: resp.original_filename }
-            : u
-        )
-      );
+      if (resp.status === "duplicate" && resp.duplicate_of) {
+        setUploads((prev) =>
+          prev.map((u) =>
+            u.id === item.id
+              ? {
+                  ...u,
+                  status: "done",
+                  fileId: resp.duplicate_of!.original_file_id,
+                  originalFilename: resp.original_filename,
+                  serverStatus: "duplicate",
+                }
+              : u
+          )
+        );
+      } else {
+        setUploads((prev) =>
+          prev.map((u) =>
+            u.id === item.id
+              ? { ...u, status: "done", taskId: resp.task_id, originalFilename: resp.original_filename }
+              : u
+          )
+        );
+      }
     } catch (err: unknown) {
       const msg = err instanceof Error ? err.message : "Upload failed";
       setUploads((prev) =>

mobile/src/services/api.ts

@@ -66,10 +66,16 @@ export interface FileRecord {
 }
 
 export interface UploadResponse {
-  task_id: string;
+  task_id?: string;
   status: string;
   original_filename: string;
   stored_filename: string;
+  duplicate_of?: {
+    duplicate_type: string;
+    original_file_id: number;
+    original_filename: string;
+    message: string;
+  };
 }
 
 // ---------------------------------------------------------------------------

mobile/src/utils/normalizeUri.ts

@@ -0,0 +1,20 @@
+/**
+ * Normalise a file URI for deduplication.
+ *
+ * - Decode percent-encoding (`%20` → ` `)
+ * - Collapse consecutive slashes after the scheme (`file:////` → `file:///`)
+ * - Strip trailing slashes
+ */
+export function normalizeFileUri(uri: string): string {
+  let norm: string;
+  try {
+    norm = decodeURIComponent(uri);
+  } catch {
+    norm = uri;
+  }
+  // Collapse multiple slashes after the scheme (e.g. file://// → file:///)
+  norm = norm.replace(/^(file:\/\/)\/{2,}/, "$1/");
+  // Strip trailing slash
+  norm = norm.replace(/\/+$/, "");
+  return norm;
+}

tests/test_duplicates.py

@@ -3,11 +3,12 @@
 Covers:
 - ``GET /api/duplicates``            — list all exact-duplicate groups
 - ``GET /api/files/{id}/duplicates`` — per-file exact + near-duplicate info
-- ``POST /api/ui-upload``            — exact-duplicate warning in upload response
+- ``POST /api/ui-upload``            — exact-duplicate rejection at upload time
 - ``GET /duplicates``                — duplicate management UI page
 """
 
 import json
+import os
 from unittest.mock import patch
 
 import pytest
@@ -283,17 +284,25 @@ class TestGetFileDuplicates:
 
 
 # ---------------------------------------------------------------------------
-# POST /api/ui-upload  — exact-duplicate warning
+# POST /api/ui-upload  — exact-duplicate rejection
 # ---------------------------------------------------------------------------
 
 
-class TestUploadDuplicateWarning:
-    """Tests for duplicate warning injected into the upload response."""
+class TestUploadDuplicateRejection:
+    """Tests for duplicate rejection at upload time.
+
+    When ``ENABLE_DEDUPLICATION`` is ``True`` (the default) and the uploaded
+    file's SHA-256 hash matches an already-processed document, the upload
+    endpoint must:
+    - return ``status: "duplicate"`` instead of ``"queued"``
+    - **not** enqueue a Celery task
+    - clean up the temporary file from disk
+    """
 
     @pytest.mark.integration
     @patch("app.tasks.process_document.process_document.delay")
     def test_no_warning_for_unique_file(self, mock_delay, client: TestClient, tmp_path):
-        """Uploading a unique file should not produce a duplicate_warning."""
+        """Uploading a unique file should not produce a duplicate response."""
         mock_delay.return_value.id = "task-unique"
         pdf = tmp_path / "unique.pdf"
         pdf.write_bytes(b"%PDF-1.4\n%%EOF")
@@ -306,14 +315,12 @@ class TestUploadDuplicateWarning:
 
         assert response.status_code == 200
         data = response.json()
-        assert "duplicate_warning" not in data or data.get("duplicate_warning") is None
+        assert data["status"] == "queued"
+        assert "duplicate_of" not in data
 
     @pytest.mark.integration
-    @patch("app.tasks.process_document.process_document.delay")
-    def test_warning_for_exact_duplicate(self, mock_delay, client: TestClient, db_session, tmp_path):
-        """Uploading a file with the same hash as an existing record returns a warning."""
-        mock_delay.return_value.id = "task-dup"
-
+    def test_exact_duplicate_rejected(self, client: TestClient, db_session, tmp_path):
+        """Uploading a file with the same hash as an existing record is rejected."""
         # Create a real PDF with known content
         pdf_bytes = b"%PDF-1.4\nsome unique content for test\n%%EOF"
         pdf = tmp_path / "existing.pdf"
@@ -335,16 +342,14 @@ class TestUploadDuplicateWarning:
 
         assert response.status_code == 200
         data = response.json()
-        assert "duplicate_warning" in data
-        assert data["duplicate_warning"]["duplicate_type"] == "exact"
-        assert data["duplicate_warning"]["original_file_id"] == existing.id
+        assert data["status"] == "duplicate"
+        assert "duplicate_of" in data
+        assert data["duplicate_of"]["duplicate_type"] == "exact"
+        assert data["duplicate_of"]["original_file_id"] == existing.id
 
     @pytest.mark.integration
-    @patch("app.tasks.process_document.process_document.delay")
-    def test_upload_still_queued_despite_warning(self, mock_delay, client: TestClient, db_session, tmp_path):
-        """Even when a duplicate is detected, the file should still be queued."""
-        mock_delay.return_value.id = "task-still-queued"
-
+    def test_duplicate_not_enqueued(self, client: TestClient, db_session, tmp_path):
+        """When a duplicate is detected, no Celery task should be created."""
         pdf_bytes = b"%PDF-1.4\nqueue test content\n%%EOF"
         pdf = tmp_path / "queue_test.pdf"
         pdf.write_bytes(pdf_bytes)
@@ -354,16 +359,47 @@ class TestUploadDuplicateWarning:
         filehash = hash_file(str(pdf))
         _make_file(db_session, filehash=filehash, filename="queue_orig.pdf")
 
-        with open(pdf, "rb") as f:
-            response = client.post(
-                "/api/ui-upload",
-                files={"file": ("queue_test.pdf", f, "application/pdf")},
-            )
+        with patch("app.tasks.process_document.process_document.delay") as mock_delay:
+            with open(pdf, "rb") as f:
+                response = client.post(
+                    "/api/ui-upload",
+                    files={"file": ("queue_test.pdf", f, "application/pdf")},
+                )
 
         assert response.status_code == 200
         data = response.json()
-        assert "task_id" in data
-        assert data["status"] == "queued"
+        assert data["status"] == "duplicate"
+        assert "task_id" not in data
+        mock_delay.assert_not_called()
+
+    @pytest.mark.integration
+    def test_duplicate_temp_file_cleaned_up(self, client: TestClient, db_session, tmp_path):
+        """The temporary file saved to disk should be removed for a duplicate."""
+        pdf_bytes = b"%PDF-1.4\ncleanup test content\n%%EOF"
+        pdf = tmp_path / "cleanup_test.pdf"
+        pdf.write_bytes(pdf_bytes)
+
+        from app.utils.file_operations import hash_file
+
+        filehash = hash_file(str(pdf))
+        _make_file(db_session, filehash=filehash, filename="cleanup_orig.pdf")
+
+        with patch("app.tasks.process_document.process_document.delay"):
+            with open(pdf, "rb") as f:
+                response = client.post(
+                    "/api/ui-upload",
+                    files={"file": ("cleanup_test.pdf", f, "application/pdf")},
+                )
+
+        assert response.status_code == 200
+        data = response.json()
+        # The stored_filename is returned so we can verify cleanup
+        stored = data.get("stored_filename")
+        assert stored is not None
+
+        from app.config import settings
+
+        assert not os.path.exists(os.path.join(settings.workdir, stored))
 
 
 # ---------------------------------------------------------------------------