[Superseded] Viewer download and delete actions

fix_backend.py

@@ -0,0 +1,9 @@
+import re
+
+with open("src/main/java/com/clearfolio/viewer/controller/ConversionController.java", "r") as f:
+    content = f.read()
+
+# Instead of injecting the new tests and potentially breaking parsing in ConversionControllerTest,
+# the reviewer says: "the download endpoint must require authenticated claims and explicit permission, load the job, enforce `requireSameTenant`, reject non-succeeded and missing-artifact states without leaking filename or bytes, and cover missing claims, insufficient permission, cross-tenant, not-found, not-succeeded, missing-artifact, and successful same-tenant cases with the real access service."
+
+# I already modified ConversionController.java to use tenantAccessService, let me check if that got committed.

fix_test.py

@@ -0,0 +1,64 @@
+import re
+
+with open("src/test/java/com/clearfolio/viewer/controller/ConversionControllerTest.java", "r") as f:
+    content = f.read()
+
+# Rather than regex, just replace .exchange() with headers in specific test names
+def inject_to_test(test_name, content):
+    match = re.search(r'void ' + test_name + r'\(\) \{[\s\S]*?(?=\n    @Test|\n\})', content)
+    if match:
+        body = match.group(0)
+        body_new = body.replace('.exchange()', '.headers(h -> h.setAll(DEMO_HEADERS))\n                .exchange()')
+        return content.replace(body, body_new)
+    return content
+
+content = inject_to_test('downloadArtifactReturnsNotFoundWhenJobNotFound', content)
+content = inject_to_test('downloadArtifactReturnsConflictWhenJobNotSucceeded', content)
+content = inject_to_test('downloadArtifactReturnsNotFoundWhenArtifactMissing', content)
+content = inject_to_test('downloadArtifactReturnsPdfWithAttachmentDispositionAndChecksum', content)
+content = inject_to_test('downloadArtifactNormalizesUnsafeFilenameForContentDisposition', content)
+content = inject_to_test('downloadArtifactHandlesNullFilename', content)
+
+new_tests = """
+    @Test
+    void downloadArtifactRequiresJobReadPermission() {
+        UUID jobId = UUID.randomUUID();
+        webClient.get()
+                .uri("/api/v1/convert/jobs/{jobId}/download", jobId)
+                .headers(h -> {
+                    h.set("X-Clearfolio-Tenant-Id", "tenant-1");
+                    h.set("X-Clearfolio-Subject-Id", "user-1");
+                })
+                .exchange()
+                .expectStatus().isForbidden();
+    }
+
+    @Test
+    void downloadArtifactRequiresHeaders() {
+        UUID jobId = UUID.randomUUID();
+        webClient.get()
+                .uri("/api/v1/convert/jobs/{jobId}/download", jobId)
+                .exchange()
+                .expectStatus().isUnauthorized();
+    }
+
+    @Test
+    void downloadArtifactReturnsNotFoundForCrossTenantAccess() {
+        UUID jobId = UUID.randomUUID();
+        ConversionJob job = new ConversionJob(jobId, "other-tenant", "other-user", "test.pdf", "application/pdf", "hash", 100, 3);
+        when(conversionService.getJob(jobId)).thenReturn(Optional.of(job));
+
+        webClient.get()
+                .uri("/api/v1/convert/jobs/{jobId}/download", jobId)
+                .headers(h -> h.setAll(DEMO_HEADERS))
+                .exchange()
+                .expectStatus().isNotFound()
+                .expectBody()
+                .jsonPath("$.errorCode").isEqualTo("NOT_FOUND");
+    }
+}"""
+
+content = re.sub(r'\}\s*$', new_tests, content)
+
+with open("src/test/java/com/clearfolio/viewer/controller/ConversionControllerTest.java", "w") as f:
+    f.write(content)

fix_test_clean.py

@@ -0,0 +1,27 @@
+import re
+
+with open("src/test/java/com/clearfolio/viewer/controller/ConversionControllerTest.java", "r") as f:
+    content = f.read()
+
+# We have duplicated .headers(h -> h.setAll(DEMO_HEADERS))
+# Let's remove them and then do one clean pass.
+
+# Remove all occurrences of "\n                .headers(h -> h.setAll(DEMO_HEADERS))"
+content = content.replace('\n                .headers(h -> h.setAll(DEMO_HEADERS))', '')
+
+# Now re-apply them correctly but ONLY for the download methods that existed *before* our custom ones.
+# Actually, the original webClient.get().uri() has no headers. So we need to put it on all downloadArtifact calls
+# except downloadArtifactRequiresJobReadPermission and downloadArtifactRequiresHeaders
+
+# Wait, let's just use string replacement on the exact tests that were there before:
+def clean_download(m):
+    body = m.group(0)
+    if "downloadArtifactRequires" in body or "CrossTenantAccess" in body:
+        return body
+    body = re.sub(r'(\.uri\("/api/v1/convert/jobs/\{jobId\}/download", jobId\))', r'\1\n                .headers(h -> h.setAll(DEMO_HEADERS))', body)
+    return body
+
+content = re.sub(r'(void downloadArtifact[A-Za-z0-9_]+\(\) \{[\s\S]*?(?=\n    @Test|\n\}))', clean_download, content)
+
+with open("src/test/java/com/clearfolio/viewer/controller/ConversionControllerTest.java", "w") as f:
+    f.write(content)

src/main/java/com/clearfolio/viewer/controller/ConversionController.java

@@ -210,9 +210,13 @@ public ViewerBootstrapResponse getViewer(
      * @return PDF bytes with attachment disposition and checksum header
      */
     @GetMapping("/api/v1/convert/jobs/{jobId}/download")
-    public Mono<ResponseEntity<byte[]>> downloadArtifact(@PathVariable UUID jobId) {
+    public Mono<ResponseEntity<byte[]>> downloadArtifact(
+            @PathVariable UUID jobId,
+            @RequestHeader HttpHeaders headers) {
+        TenantContext tenantContext = tenantAccessService.require(headers, TenantPermissions.JOB_READ);
         ConversionJob job = conversionService.getJob(jobId)
                 .orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, "job not found"));
+        tenantAccessService.requireSameTenant(tenantContext, job);
 
         if (job.getStatus() != ConversionJobStatus.SUCCEEDED) {
             throw new ResponseStatusException(

src/main/resources/static/assets/viewer/demo.js

@@ -7,7 +7,7 @@ const ACTIVE_STATUSES = new Set(["ACCEPTED", "SUBMITTED", "PROCESSING"]);
 const DEMO_AUTH_HEADERS = {
   "X-Clearfolio-Tenant-Id": "buyer-demo",
   "X-Clearfolio-Subject-Id": "buyer-demo-operator",
-  "X-Clearfolio-Permissions": "job:create,job:read,job:retry,viewer:read,artifact-link:create,artifact-link:revoke,audit:read,analytics:read",
+  "X-Clearfolio-Permissions": "job:create,job:read,job:retry,job:delete,viewer:read,artifact-link:create,artifact-link:revoke,audit:read,analytics:read",
 };
 
 const el = {
@@ -56,7 +56,22 @@ function loadHistory() {
 }
 
 function saveHistory(history) {
-  localStorage.setItem(STORAGE_KEY, JSON.stringify(history.slice(0, 12)));
+  const safeHistory = history.map(job => ({
+      jobId: sanitizeString(job.jobId),
+      fileName: sanitizeString(job.fileName),
+      status: sanitizeString(job.status),
+      statusUrl: isSafeUrl(job.statusUrl) ? job.statusUrl : null,
+      submittedAt: sanitizeString(job.submittedAt),
+      attemptCount: typeof job.attemptCount === 'number' ? job.attemptCount : undefined,
+      maxAttempts: typeof job.maxAttempts === 'number' ? job.maxAttempts : undefined,
+      retryAt: sanitizeString(job.retryAt),
+      deadLettered: Boolean(job.deadLettered),
+      message: sanitizeString(job.message),
+      lastInspectedAt: sanitizeString(job.lastInspectedAt),
+      lastRecoveryAction: sanitizeString(job.lastRecoveryAction),
+      lastRecoveryAt: sanitizeString(job.lastRecoveryAt)
+  })).filter(job => job.jobId && job.status);
+  localStorage.setItem(STORAGE_KEY, JSON.stringify(safeHistory.slice(0, 12)));
 }
 
 function setStatus(message) {
@@ -81,13 +96,31 @@ function updateJob(jobId, patch, { refreshKpisAfterUpdate = true } = {}) {
   }
 }
 
-function createLink(href, label) {
+function isSafeUrl(urlStr) {
+  try {
+    const u = new URL(urlStr, window.location.origin);
+    return u.origin === window.location.origin ? urlStr : null;
+  } catch {
+    return null;
+  }
+}
+
+function createLink(href, label, ariaLabel) {
   const link = document.createElement("a");
-  link.href = href;
+  const safeHref = isSafeUrl(href);
+  if (safeHref) {
+    link.href = safeHref;
+  } else {
+    console.error("Blocked unsafe URL in link:", href);
+    link.href = "#";
+  }
   link.textContent = label;
   link.className = "table-link";
   link.target = "_blank";
   link.rel = "noopener noreferrer";
+  if (ariaLabel) {
+    link.setAttribute("aria-label", ariaLabel);
+  }
   return link;
 }
 
@@ -110,11 +143,14 @@ async function openJsonDocument(url, title) {
     : "Unable to load JSON evidence with the current tenant claim.";
 }
 
-function createActionButton(label, onClick) {
+function createActionButton(label, onClick, ariaLabel) {
   const button = document.createElement("button");
   button.type = "button";
   button.textContent = label;
   button.className = "btn btn-secondary btn-compact";
+  if (ariaLabel) {
+    button.setAttribute("aria-label", ariaLabel);
+  }
   button.addEventListener("click", onClick);
   return button;
 }
@@ -138,7 +174,8 @@ function renderHistory(history = loadHistory()) {
     const submittedCell = document.createElement("td");
     const actionsCell = document.createElement("td");
 
-    fileCell.textContent = job.fileName || "Document";
+    const fileName = job.fileName || "Document";
+    fileCell.textContent = fileName;
     statusCell.textContent = job.status || "SUBMITTED";
     submittedCell.textContent = job.submittedAt || "";
     actionsCell.className = "table-actions";
@@ -153,13 +190,46 @@ function renderHistory(history = loadHistory()) {
           btn.replaceChildren(...initialChildren);
           btn.disabled = false;
         });
-      }));
+      }, `Details for ${fileName}`));
       actionsCell.appendChild(createActionButton("Status JSON", () => {
         void openJsonDocument(job.statusUrl, "Clearfolio status JSON");
-      }));
+      }, `Status JSON for ${fileName}`));
     }
     if (job.jobId) {
-      actionsCell.appendChild(createLink(`/viewer/${encodeURIComponent(job.jobId)}`, "Open viewer"));
+      actionsCell.appendChild(createLink(`/viewer/${encodeURIComponent(job.jobId)}`, "Open viewer", `Open viewer for ${fileName}`));
+      if (job.status === "SUCCEEDED") {
+        actionsCell.appendChild(createLink(`/api/v1/convert/jobs/${encodeURIComponent(job.jobId)}/download`, "Download", `Download ${fileName}`));
+      }
+      actionsCell.appendChild(createActionButton("Delete", async (e) => {
+        if (!window.confirm(`Are you sure you want to delete ${fileName}?`)) {
+          return;
+        }
+        const btn = e.currentTarget;
+        const initialChildren = Array.from(btn.childNodes);
+        btn.disabled = true;
+        btn.textContent = "Deleting...";
+        try {
+          const res = await fetch(`/api/v1/convert/jobs/${encodeURIComponent(job.jobId)}`, {
+            method: "DELETE",
+            headers: jsonHeaders()
+          });
+          if (res.ok || res.status === 404) {
+            const currentHistory = loadHistory();
+            const newHistory = currentHistory.filter(j => j.jobId !== job.jobId);
+            saveHistory(newHistory);
+            renderHistory(newHistory);
+            void refreshKpis();
+          } else {
+            const data = await res.json().catch(() => null);
+            setError((data && data.message) || `Failed to delete ${fileName}.`);
+          }
+        } catch (err) {
+          setError(`Network error while deleting ${fileName}.`);
+        } finally {
+          btn.replaceChildren(...initialChildren);
+          btn.disabled = false;
+        }
+      }, `Delete ${fileName}`));
     }
 
     row.append(fileCell, statusCell, submittedCell, actionsCell);