From 5eee1e4a6911c4f7b3ac4eed7b4f71551e75ef95 Mon Sep 17 00:00:00 2001
From: Ibrahim Chehab <chehab.ibrahim04@gmail.com>
Date: Tue, 25 Aug 2026 21:27:52 -0400
Subject: [PATCH] Return presigned URLs from UploadDto and honour the anon flag

UploadDto.fileUrl was the verbatim R2 object key. Verified live as an
ordinary ROLE_USER, GET /api/user/uploads returned:

  {"fileUrl":"images/e5c6f37c-...","uploadedBy":"f7b1d402-...",
   "approved":true,"anon":true}

Two defects in one response. The bucket is fronted by a public custom
domain (PosterService serves publicUrl + "/" + objectKey unsigned), so
a leaked key is a direct unauthenticated fetch of the media -- and
AdminUploadDto deliberately routes through R2StorageService.getSecureUrl,
which throws SecurityException for unapproved content. The user-facing
path skipped that boundary entirely. Separately, uploadedBy was
serialized even when anon=true, revoking the anonymity the uploader was
promised at submit time.

fileUrl and thumbnailUrl are now absolute expiring presigned URLs
issued through the approval gate, with urlExpiresAt so clients re-fetch
rather than cache. uploadedBy and instagramHandle are null on anonymous
uploads unless the caller is the uploader or holds media:upload:read.

Breaking change for utmmsa-pwa; see CLIENT_MIGRATION.md.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011xQusSt39vP3c82gAUXRCm
---
 CLIENT_MIGRATION.md                           | 121 ++++++++
 .../dto/upload/response/UploadDto.java        |  33 ++
 .../lensbridge/service/R2StorageService.java  |  10 +
 .../lensbridge/service/UploadService.java     | 141 +++++++--
 .../R2StorageServiceSecureUrlTest.java        | 113 +++++++
 .../service/UploadServiceDtoMappingTest.java  | 282 ++++++++++++++++++
 6 files changed, 681 insertions(+), 19 deletions(-)
 create mode 100644 CLIENT_MIGRATION.md
 create mode 100644 src/test/java/com/ibrasoft/lensbridge/service/R2StorageServiceSecureUrlTest.java
 create mode 100644 src/test/java/com/ibrasoft/lensbridge/service/UploadServiceDtoMappingTest.java

diff --git a/CLIENT_MIGRATION.md b/CLIENT_MIGRATION.md
new file mode 100644
index 0000000..3e85ace
--- /dev/null
+++ b/CLIENT_MIGRATION.md
@@ -0,0 +1,121 @@
+# Client migration — `UploadDto` no longer returns raw storage keys
+
+**Affects:** `C:\Code\utmmsa-pwa` (and any other consumer of the generated client)
+**Breaking:** yes — the *values* of `fileUrl` and `thumbnailUrl` change meaning, and `uploadedBy` /
+`instagramHandle` can now be `null` where they previously were not.
+
+## Endpoints affected
+
+Every endpoint returning `UploadDto` or `Page<UploadDto>`:
+
+| Method | Path | Handler |
+| --- | --- | --- |
+| GET | `/api/user/uploads` | `UserController.getUserUploads` |
+| GET | `/api/upload/{uploadId}` | `FileUploadController.getUploadById` |
+| GET | `/api/upload/event/{eventId}` | `FileUploadController.getUploadsByEvent` |
+
+`AdminUploadDto` (the `/api/admin/uploads/**` family) is **unchanged** — it already returned presigned
+URLs in `secureUrl` / `thumbnailUrl`.
+
+## What changed
+
+### 1. `fileUrl` and `thumbnailUrl` are now absolute presigned URLs
+
+Before — the verbatim R2 object key:
+
+```json
+{ "fileUrl": "images/e5c6f37c-845d-4252-9a2f-16e5e68eee96",
+  "thumbnailUrl": "thumbnails/e5c6f37c-845d-4252-9a2f-16e5e68eee96" }
+```
+
+After — a scoped, signed, expiring URL on the media domain:
+
+```json
+{ "fileUrl": "https://media.lensbridge.tech/images/e5c6f37c-…?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Expires=900&X-Amz-Signature=…",
+  "thumbnailUrl": "https://media.lensbridge.tech/thumbnails/e5c6f37c-…?…&X-Amz-Signature=…",
+  "urlExpiresAt": "2026-08-25T18:45:00Z" }
+```
+
+**Why:** `media.lensbridge.tech` is a public custom domain in front of the bucket (see
+`PosterService.java:100,181`, which serves `publicUrl + "/" + objectKey` unsigned). Returning the key
+was therefore equivalent to returning the media itself, to anyone who obtained the JSON, forever, with
+no approval check. The presign step is also where the approval gate lives —
+`R2StorageService.getSecureUrl` refuses unapproved content to unprivileged callers.
+
+**Frontend change required:** stop concatenating. Anywhere the PWA does
+
+```ts
+const src = `${MEDIA_BASE_URL}/${upload.fileUrl}`;      // ← delete this
+```
+
+use the value directly:
+
+```ts
+const src = upload.fileUrl;                              // already absolute
+```
+
+Grep the PWA for the media base-URL constant (`VITE_MEDIA_URL` / `media.lensbridge.tech` /
+any `${...}/${upload.fileUrl}` template) and remove the join at every call site, including
+`<img src>`, `<video src>`, any `srcset`, download links, and share/copy-link actions.
+
+### 2. Those URLs expire — do not cache them
+
+The signature is valid for `cloudflare.r2.url-expiration-minutes` (default **15 minutes**). A new
+field, `urlExpiresAt` (ISO-8601 instant, nullable), tells you exactly when.
+
+Consequences for the PWA:
+
+- **Do not persist `fileUrl` in localStorage / IndexedDB / a service-worker cache / a Redux store that
+  outlives the session.** A rehydrated URL past `urlExpiresAt` returns 403 and renders a broken image.
+- **Do not put the URL in a React `key`, a cache key, or a dedup key** — it changes on every fetch of
+  the same upload. Use `uuid` for identity.
+- Refetch the upload (or the page of uploads) when `urlExpiresAt` passes and the media is still on
+  screen. A long-lived gallery tab is the realistic failure case; a simple approach is to refetch the
+  current page when `Date.now() > urlExpiresAt - 60_000` on tab focus.
+- Image `onError` handlers should retry once *after refetching the DTO*, not by retrying the same URL.
+
+### 3. `fileUrl` / `thumbnailUrl` can be `null`
+
+`null` means "you may not view these bytes" — the upload is unapproved and you are neither its
+uploader nor a moderator — or the storage backend failed to sign. Previously the key was always
+present regardless of approval state.
+
+**Frontend change required:** render a placeholder / "Pending review" state when `fileUrl` is null.
+`upload.approved === false` is the expected companion signal. Do not `String(upload.fileUrl)` or feed
+`null` into an `<img src>`.
+
+`thumbnailUrl` still falls back to the full-size URL when an upload has no thumbnail, so a non-null
+`fileUrl` implies a non-null `thumbnailUrl`.
+
+### 4. `uploadedBy` and `instagramHandle` are `null` on anonymous uploads
+
+When `anon === true`, both fields are nulled unless the caller is the uploader themselves or holds
+the `media:upload:read` permission. Previously `uploadedBy` was serialized unconditionally, which
+silently voided the anonymity the uploader was promised at submit time — a `ROLE_USER` listing an
+event's uploads could deanonymise every anonymous submission in it.
+
+**Frontend change required:** anywhere the PWA reads `upload.uploadedBy` or `upload.instagramHandle`
+for display, attribution, "is this mine?" checks, or grouping, guard for `null`. Note that on
+`/api/user/uploads` the caller *is* the uploader, so those fields stay populated there — the change
+bites on `/api/upload/event/{eventId}`.
+
+## Type / generated-client updates
+
+`UploadDto` gains one optional field:
+
+```ts
+urlExpiresAt?: string;   // ISO-8601 instant; null when no URLs were issued
+```
+
+and `fileUrl`, `thumbnailUrl`, `uploadedBy`, `instagramHandle` must all be treated as nullable.
+Regenerate the client from the updated `openapi.yaml` at the repo root.
+
+## Suggested rollout
+
+1. Ship the frontend change that *tolerates* absolute URLs (`fileUrl.startsWith('http') ? fileUrl :
+   join(base, fileUrl)`) and null media — this is compatible with both old and new backends.
+2. Deploy the backend.
+3. Delete the compatibility branch and the media base-URL constant.
+
+Step 1 is optional if backend and PWA deploy together; if they do not, skip it and you will serve
+`https://media.lensbridge.tech/https://media.lensbridge.tech/...` for the duration of the skew.
diff --git a/src/main/java/com/ibrasoft/lensbridge/dto/upload/response/UploadDto.java b/src/main/java/com/ibrasoft/lensbridge/dto/upload/response/UploadDto.java
index e8862e9..3dcb52f 100644
--- a/src/main/java/com/ibrasoft/lensbridge/dto/upload/response/UploadDto.java
+++ b/src/main/java/com/ibrasoft/lensbridge/dto/upload/response/UploadDto.java
@@ -1,6 +1,7 @@
 package com.ibrasoft.lensbridge.dto.upload.response;
 
 import com.ibrasoft.lensbridge.model.upload.UploadType;
+import io.swagger.v3.oas.annotations.media.Schema;
 import lombok.AllArgsConstructor;
 import lombok.Data;
 import lombok.NoArgsConstructor;
@@ -8,17 +9,45 @@
 import java.time.Instant;
 import java.util.UUID;
 
+/**
+ * The user-facing view of an upload.
+ * <p>
+ * Two things about this DTO are security boundaries, not cosmetics:
+ * <ul>
+ *   <li>{@link #fileUrl} and {@link #thumbnailUrl} are <strong>absolute, expiring, presigned
+ *       URLs</strong>, never the raw R2 object key. The bucket is fronted by a public custom
+ *       domain, so a leaked key is an unauthenticated fetch of the media; and the presign step
+ *       is where the approval gate is enforced. Both are {@code null} when the caller is not
+ *       allowed to see the bytes (unapproved content, or a storage failure).</li>
+ *   <li>{@link #uploadedBy} and {@link #instagramHandle} are {@code null} on anonymous uploads
+ *       unless the caller is the uploader or holds {@code media:upload:read}. Anonymity was
+ *       promised to the uploader at submit time; serializing the author id revokes it.</li>
+ * </ul>
+ */
 @Data
 @NoArgsConstructor
 @AllArgsConstructor
 public class UploadDto {
     private UUID uuid;
     private String fileName;
+
+    @Schema(description = "Absolute, expiring presigned URL for the media. Use verbatim -- do NOT "
+            + "concatenate it with a media base URL. Null when the caller may not view the bytes.",
+            example = "https://media.lensbridge.tech/images/e5c6f37c-...?X-Amz-Signature=...")
     private String fileUrl;
+
+    @Schema(description = "Absolute, expiring presigned URL for the thumbnail. Falls back to the "
+            + "full-size URL when no thumbnail exists. Null when the caller may not view the bytes.")
     private String thumbnailUrl;
+
     private String uploadDescription;
+
+    @Schema(description = "Null on anonymous uploads unless the caller is the uploader or holds media:upload:read.")
     private String instagramHandle;
+
+    @Schema(description = "Null on anonymous uploads unless the caller is the uploader or holds media:upload:read.")
     private UUID uploadedBy;
+
     private UUID eventId;
     private String eventName;
     private Instant createdDate;
@@ -26,4 +55,8 @@ public class UploadDto {
     private boolean featured;
     private boolean isAnon;
     private UploadType contentType;
+
+    @Schema(description = "When fileUrl/thumbnailUrl stop working. Re-fetch the upload after this "
+            + "instant rather than caching the URLs. Null when no URLs were issued.")
+    private Instant urlExpiresAt;
 }
diff --git a/src/main/java/com/ibrasoft/lensbridge/service/R2StorageService.java b/src/main/java/com/ibrasoft/lensbridge/service/R2StorageService.java
index 03e549a..886e20e 100644
--- a/src/main/java/com/ibrasoft/lensbridge/service/R2StorageService.java
+++ b/src/main/java/com/ibrasoft/lensbridge/service/R2StorageService.java
@@ -344,6 +344,16 @@ public String calculateSha256Hash(String objectKey) throws Exception {
         return HexFormat.of().formatHex(digest.digest(bytes));
     }
 
+    /**
+     * How long a presigned download URL stays valid.
+     * <p>
+     * Exposed so callers that hand a presigned URL to a client can also tell the client
+     * when it dies, instead of leaving it to discover the expiry via a 403.
+     */
+    public Duration getDownloadUrlExpiration() {
+        return Duration.ofMinutes(urlExpirationMinutes);
+    }
+
     /**
      * Generate a presigned download URL for an object, with access control.
      * Throws SecurityException if the content is unapproved and the caller is not admin.
diff --git a/src/main/java/com/ibrasoft/lensbridge/service/UploadService.java b/src/main/java/com/ibrasoft/lensbridge/service/UploadService.java
index 6026278..c25b0f0 100644
--- a/src/main/java/com/ibrasoft/lensbridge/service/UploadService.java
+++ b/src/main/java/com/ibrasoft/lensbridge/service/UploadService.java
@@ -4,6 +4,7 @@
 import com.ibrasoft.lensbridge.dto.upload.response.UploadDto;
 import com.ibrasoft.lensbridge.dto.auth.response.UserStatsResponse;
 import com.ibrasoft.lensbridge.exception.FileProcessingException;
+import com.ibrasoft.lensbridge.model.auth.Permission;
 import com.ibrasoft.lensbridge.model.auth.User;
 import com.ibrasoft.lensbridge.model.minbar.BoardEvent;
 import com.ibrasoft.lensbridge.model.upload.Upload;
@@ -16,6 +17,9 @@
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.Pageable;
+import org.springframework.security.authentication.AnonymousAuthenticationToken;
+import org.springframework.security.core.Authentication;
+import org.springframework.security.core.context.SecurityContextHolder;
 import org.springframework.stereotype.Service;
 
 import java.time.Instant;
@@ -154,7 +158,8 @@ public Optional<Upload> getUploadById(UUID id) {
     }
 
     public Optional<UploadDto> getUploadByIdAsDto(UUID id) {
-        return uploadRepository.findById(id).map(this::toUploadDto);
+        UploadViewer viewer = currentViewer();
+        return uploadRepository.findById(id).map(upload -> toUploadDto(upload, viewer));
     }
 
     public Page<Upload> getAllUploads(Pageable pageable) {
@@ -170,13 +175,17 @@ public Page<Upload> getUploadsByEvent(UUID eventId, Pageable pageable) {
     public Page<UploadDto> getUploadsByEventAsDto(UUID eventId, Pageable pageable) {
         BoardEvent boardEvent = boardEventRepository.findById(eventId)
                 .orElseThrow(() -> new IllegalArgumentException("Event not found"));
-        return uploadRepository.findByBoardEventAndDeletedAtIsNull(boardEvent, pageable).map(this::toUploadDto);
+        UploadViewer viewer = currentViewer();
+        return uploadRepository.findByBoardEventAndDeletedAtIsNull(boardEvent, pageable)
+                .map(upload -> toUploadDto(upload, viewer));
     }
 
     public Page<UploadDto> getUserUploads(UUID userId, Pageable pageable) {
         User user = userService.findById(userId)
                 .orElseThrow(() -> new IllegalArgumentException("User not found"));
-        return uploadRepository.findByUploadedByAndDeletedAtIsNull(user, pageable).map(this::toUploadDto);
+        UploadViewer viewer = currentViewer();
+        return uploadRepository.findByUploadedByAndDeletedAtIsNull(user, pageable)
+                .map(upload -> toUploadDto(upload, viewer));
     }
 
     public Page<AdminUploadDto> getAllUploadsForAdmin(Pageable pageable) {
@@ -222,23 +231,117 @@ private Upload findRequiredById(UUID id) {
                 .orElseThrow(() -> new IllegalArgumentException("Upload not found: " + id));
     }
 
+    /**
+     * Who is asking for a {@link UploadDto}, and what they are allowed to see.
+     * <p>
+     * Resolved once per query rather than once per row: a page of 50 uploads should cost one
+     * principal lookup, not 50.
+     *
+     * @param userId            the caller's account id, or {@code null} for an unauthenticated caller
+     * @param canReadAllUploads whether the caller holds {@code media:upload:read}
+     */
+    record UploadViewer(UUID userId, boolean canReadAllUploads) {
+
+        static UploadViewer anonymous() {
+            return new UploadViewer(null, false);
+        }
+
+        /** True when this viewer is the account that submitted the upload. */
+        boolean isUploader(Upload upload) {
+            return userId != null
+                    && upload.getUploadedBy() != null
+                    && userId.equals(upload.getUploadedBy().getId());
+        }
+
+        /**
+         * Whether the uploader's identity may be disclosed on an anonymous upload, and whether
+         * unapproved bytes may be presigned. Moderators need both to do review; the uploader
+         * needs both to see their own pending submission.
+         */
+        boolean isPrivilegedFor(Upload upload) {
+            return canReadAllUploads || isUploader(upload);
+        }
+    }
+
+    /**
+     * Resolve the caller from the security context. Never throws: an unresolvable principal
+     * degrades to the least-privileged viewer rather than failing the request.
+     */
+    UploadViewer currentViewer() {
+        Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
+        if (authentication == null || !authentication.isAuthenticated()
+                || authentication instanceof AnonymousAuthenticationToken) {
+            return UploadViewer.anonymous();
+        }
+
+        boolean canReadAll = authentication.getAuthorities().stream()
+                .anyMatch(a -> Permission.Authority.MEDIA_UPLOAD_READ.equals(a.getAuthority()));
+
+        UUID userId = userService.findByEmail(authentication.getName())
+                .map(User::getId)
+                .orElse(null);
+
+        return new UploadViewer(userId, canReadAll);
+    }
+
     // TODO: Consolidate this in UploadDTO class as from(Upload)
-    private UploadDto toUploadDto(Upload upload) {
-        return new UploadDto(
-                upload.getUuid(),
-                upload.getFileName(),
-                upload.getFileUrl(),
-                upload.getThumbnailUrl(),
-                upload.getUploadDescription(),
-                upload.getInstagramHandle(),
-                upload.getUploadedBy() != null ? upload.getUploadedBy().getId() : null,
-                upload.getBoardEvent() != null ? upload.getBoardEvent().getId() : null,
-                upload.getBoardEvent() != null ? upload.getBoardEvent().getName() : null,
-                upload.getCreatedDate(),
-                upload.isApproved(),
-                upload.isFeatured(),
-                upload.isAnon(),
-                upload.getContentType());
+    UploadDto toUploadDto(Upload upload, UploadViewer viewer) {
+        boolean privileged = viewer.isPrivilegedFor(upload);
+
+        UploadDto dto = new UploadDto();
+        dto.setUuid(upload.getUuid());
+        dto.setFileName(upload.getFileName());
+        dto.setUploadDescription(upload.getUploadDescription());
+        dto.setEventId(upload.getBoardEvent() != null ? upload.getBoardEvent().getId() : null);
+        dto.setEventName(upload.getBoardEvent() != null ? upload.getBoardEvent().getName() : null);
+        dto.setCreatedDate(upload.getCreatedDate());
+        dto.setApproved(upload.isApproved());
+        dto.setFeatured(upload.isFeatured());
+        dto.setAnon(upload.isAnon());
+        dto.setContentType(upload.getContentType());
+
+        // Anonymity is a promise made to the uploader at submit time. Only the uploader
+        // themselves and moderators (media:upload:read) get to see behind it.
+        boolean masked = upload.isAnon() && !privileged;
+        dto.setUploadedBy(masked || upload.getUploadedBy() == null
+                ? null
+                : upload.getUploadedBy().getId());
+        dto.setInstagramHandle(masked ? null : upload.getInstagramHandle());
+
+        applySecureUrls(dto, upload, privileged);
+        return dto;
+    }
+
+    /**
+     * Replace the stored R2 object keys with scoped, expiring presigned URLs.
+     * <p>
+     * The keys themselves never leave the server: the bucket is fronted by a public custom
+     * domain, so handing out a key is handing out the media. {@code getSecureUrl} is also where
+     * the approval gate lives -- it refuses unapproved content to unprivileged callers. A refusal
+     * (or any storage failure) nulls the URLs instead of failing the whole listing.
+     */
+    private void applySecureUrls(UploadDto dto, Upload upload, boolean privileged) {
+        try {
+            String fileKey = r2StorageService.extractObjectKey(upload.getFileUrl());
+            if (fileKey == null) {
+                return;
+            }
+
+            String fileUrl = r2StorageService.getSecureUrl(fileKey, upload.isApproved(), privileged);
+
+            String thumbKey = r2StorageService.extractObjectKey(upload.getThumbnailUrl());
+            String thumbUrl = thumbKey != null
+                    ? r2StorageService.getSecureThumbnailUrl(thumbKey, upload.isApproved(), privileged)
+                    : fileUrl;
+
+            dto.setFileUrl(fileUrl);
+            dto.setThumbnailUrl(thumbUrl);
+            dto.setUrlExpiresAt(Instant.now().plus(r2StorageService.getDownloadUrlExpiration()));
+        } catch (SecurityException e) {
+            log.debug("Withholding media URLs for upload {}: {}", upload.getUuid(), e.getMessage());
+        } catch (Exception e) {
+            log.error("Failed to generate secure URLs for upload {}: {}", upload.getUuid(), e.getMessage());
+        }
     }
 
     private AdminUploadDto toAdminDto(Upload upload) {
diff --git a/src/test/java/com/ibrasoft/lensbridge/service/R2StorageServiceSecureUrlTest.java b/src/test/java/com/ibrasoft/lensbridge/service/R2StorageServiceSecureUrlTest.java
new file mode 100644
index 0000000..0db9cd5
--- /dev/null
+++ b/src/test/java/com/ibrasoft/lensbridge/service/R2StorageServiceSecureUrlTest.java
@@ -0,0 +1,113 @@
+package com.ibrasoft.lensbridge.service;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+import org.springframework.test.util.ReflectionTestUtils;
+import software.amazon.awssdk.services.s3.S3Client;
+import software.amazon.awssdk.services.s3.presigner.S3Presigner;
+import software.amazon.awssdk.services.s3.presigner.model.GetObjectPresignRequest;
+import software.amazon.awssdk.services.s3.presigner.model.PresignedGetObjectRequest;
+
+import java.net.URI;
+import java.time.Duration;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+/**
+ * The approval gate and the shape of the URL handed to clients. R2 is not reachable from CI,
+ * so the presigner is mocked and we assert on what we ask it for and what we do with its answer.
+ */
+@ExtendWith(MockitoExtension.class)
+class R2StorageServiceSecureUrlTest {
+
+    private static final String SIGNED_ORIGIN_URL =
+            "https://account.r2.cloudflarestorage.com/lensbridge-media/images/abc-123"
+                    + "?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Expires=900&X-Amz-Signature=deadbeef";
+
+    @Mock
+    private S3Client s3Client;
+
+    @Mock
+    private S3Presigner presigner;
+
+    private R2StorageService service;
+
+    @BeforeEach
+    void setUp() {
+        service = new R2StorageService(s3Client, presigner);
+        ReflectionTestUtils.setField(service, "bucketName", "lensbridge-media");
+        ReflectionTestUtils.setField(service, "publicUrl", "https://media.lensbridge.tech");
+        ReflectionTestUtils.setField(service, "urlExpirationMinutes", 15L);
+    }
+
+    private void stubPresigner() {
+        PresignedGetObjectRequest presigned = mock(PresignedGetObjectRequest.class);
+        try {
+            when(presigned.url()).thenReturn(URI.create(SIGNED_ORIGIN_URL).toURL());
+        } catch (Exception e) {
+            throw new IllegalStateException(e);
+        }
+        when(presigner.presignGetObject(any(GetObjectPresignRequest.class))).thenReturn(presigned);
+    }
+
+    @Test
+    void secureUrlIsAbsoluteSignedAndExpiring() {
+        stubPresigner();
+
+        String url = service.getSecureUrl("images/abc-123", true, false);
+
+        assertThat(url).startsWith("https://media.lensbridge.tech/images/abc-123");
+        assertThat(url).contains("X-Amz-Signature=");
+        assertThat(url).contains("X-Amz-Expires=");
+        // The bucket name must not survive the rewrite to the public domain.
+        assertThat(url).doesNotContain("lensbridge-media/");
+    }
+
+    @Test
+    void secureUrlRequestsTheConfiguredExpiryForTheRightKey() {
+        stubPresigner();
+
+        service.getSecureUrl("images/abc-123", true, false);
+
+        ArgumentCaptor<GetObjectPresignRequest> captor =
+                ArgumentCaptor.forClass(GetObjectPresignRequest.class);
+        verify(presigner).presignGetObject(captor.capture());
+
+        GetObjectPresignRequest request = captor.getValue();
+        assertThat(request.signatureDuration()).isEqualTo(Duration.ofMinutes(15));
+        assertThat(request.getObjectRequest().key()).isEqualTo("images/abc-123");
+        assertThat(request.getObjectRequest().bucket()).isEqualTo("lensbridge-media");
+    }
+
+    @Test
+    void unapprovedContentIsRefusedToUnprivilegedCallersWithoutEverPresigning() {
+        assertThatThrownBy(() -> service.getSecureUrl("images/abc-123", false, false))
+                .isInstanceOf(SecurityException.class);
+        assertThatThrownBy(() -> service.getSecureThumbnailUrl("thumbnails/abc-123", false, false))
+                .isInstanceOf(SecurityException.class);
+
+        verify(presigner, never()).presignGetObject(any(GetObjectPresignRequest.class));
+    }
+
+    @Test
+    void unapprovedContentIsPresignedForPrivilegedCallers() {
+        stubPresigner();
+
+        assertThat(service.getSecureUrl("images/abc-123", false, true)).contains("X-Amz-Signature=");
+    }
+
+    @Test
+    void downloadUrlExpirationReflectsConfiguration() {
+        assertThat(service.getDownloadUrlExpiration()).isEqualTo(Duration.ofMinutes(15));
+    }
+}
diff --git a/src/test/java/com/ibrasoft/lensbridge/service/UploadServiceDtoMappingTest.java b/src/test/java/com/ibrasoft/lensbridge/service/UploadServiceDtoMappingTest.java
new file mode 100644
index 0000000..3023e25
--- /dev/null
+++ b/src/test/java/com/ibrasoft/lensbridge/service/UploadServiceDtoMappingTest.java
@@ -0,0 +1,282 @@
+package com.ibrasoft.lensbridge.service;
+
+import com.ibrasoft.lensbridge.dto.upload.response.UploadDto;
+import com.ibrasoft.lensbridge.model.auth.Permission;
+import com.ibrasoft.lensbridge.model.auth.User;
+import com.ibrasoft.lensbridge.model.minbar.BoardEvent;
+import com.ibrasoft.lensbridge.model.upload.Upload;
+import com.ibrasoft.lensbridge.model.upload.UploadType;
+import com.ibrasoft.lensbridge.repository.sql.BoardEventRepository;
+import com.ibrasoft.lensbridge.repository.upload.UploadRepository;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+import org.mockito.junit.jupiter.MockitoSettings;
+import org.mockito.quality.Strictness;
+import org.springframework.security.authentication.AnonymousAuthenticationToken;
+import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
+import org.springframework.security.core.authority.SimpleGrantedAuthority;
+import org.springframework.security.core.context.SecurityContextHolder;
+
+import java.time.Duration;
+import java.time.Instant;
+import java.util.List;
+import java.util.Optional;
+import java.util.UUID;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyBoolean;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.when;
+
+/**
+ * Unit 2: {@link UploadDto} must never carry a raw R2 object key, and must never de-anonymise
+ * an upload whose author asked to stay anonymous.
+ */
+@ExtendWith(MockitoExtension.class)
+@MockitoSettings(strictness = Strictness.LENIENT)
+class UploadServiceDtoMappingTest {
+
+    private static final String FILE_KEY = "images/e5c6f37c-845d-4252-9a2f-16e5e68eee96";
+    private static final String THUMB_KEY = "thumbnails/e5c6f37c-845d-4252-9a2f-16e5e68eee96";
+
+    private final UUID uploaderId = UUID.randomUUID();
+    private final UUID strangerId = UUID.randomUUID();
+
+    @Mock
+    private UploadRepository uploadRepository;
+    @Mock
+    private UserService userService;
+    @Mock
+    private BoardEventRepository boardEventRepository;
+    @Mock
+    private R2StorageService r2StorageService;
+
+    private UploadService service;
+
+    @BeforeEach
+    void setUp() {
+        service = new UploadService(uploadRepository, userService, boardEventRepository, r2StorageService);
+
+        // extractObjectKey is a pass-through for bare keys; mirror that.
+        when(r2StorageService.extractObjectKey(any())).thenAnswer(inv -> {
+            String v = inv.getArgument(0);
+            return (v == null || v.isBlank()) ? null : v;
+        });
+        when(r2StorageService.getDownloadUrlExpiration()).thenReturn(Duration.ofMinutes(15));
+
+        // Emulate the real approval gate so the test exercises it rather than assuming it.
+        when(r2StorageService.getSecureUrl(anyString(), anyBoolean(), anyBoolean()))
+                .thenAnswer(inv -> presign(inv.getArgument(0), inv.getArgument(1), inv.getArgument(2)));
+        when(r2StorageService.getSecureThumbnailUrl(anyString(), anyBoolean(), anyBoolean()))
+                .thenAnswer(inv -> presign(inv.getArgument(0), inv.getArgument(1), inv.getArgument(2)));
+    }
+
+    private static String presign(String key, boolean approved, boolean privileged) {
+        if (!approved && !privileged) {
+            throw new SecurityException("Access denied: content is not approved");
+        }
+        return "https://media.lensbridge.tech/" + key + "?X-Amz-Expires=900&X-Amz-Signature=deadbeef";
+    }
+
+    @AfterEach
+    void clearContext() {
+        SecurityContextHolder.clearContext();
+    }
+
+    private User user(UUID id, String email) {
+        User u = new User();
+        u.setId(id);
+        u.setEmail(email);
+        u.setFirstName("Test");
+        u.setLastName("User");
+        return u;
+    }
+
+    private Upload anonUpload(boolean approved) {
+        Upload upload = new Upload();
+        upload.setUuid(UUID.randomUUID());
+        upload.setFileName("photo.jpg");
+        upload.setFileUrl(FILE_KEY);
+        upload.setThumbnailUrl(THUMB_KEY);
+        upload.setInstagramHandle("@secret_handle");
+        upload.setUploadedBy(user(uploaderId, "uploader@example.com"));
+        upload.setBoardEvent(BoardEvent.builder().id(UUID.randomUUID()).name("Jumuah").build());
+        upload.setCreatedDate(Instant.now());
+        upload.setApproved(approved);
+        upload.setAnon(true);
+        upload.setContentType(UploadType.IMAGE);
+        return upload;
+    }
+
+    private void authenticateAs(String email, String... authorities) {
+        SecurityContextHolder.getContext().setAuthentication(
+                new UsernamePasswordAuthenticationToken(email, null,
+                        List.of(authorities).stream().map(SimpleGrantedAuthority::new).toList()));
+    }
+
+    // ── URL leakage ───────────────────────────────────────────────────────────
+
+    @Test
+    void fileAndThumbnailUrlsArePresignedNotRawKeys() {
+        UploadDto dto = service.toUploadDto(anonUpload(true),
+                new UploadService.UploadViewer(strangerId, false));
+
+        assertThat(dto.getFileUrl()).isNotEqualTo(FILE_KEY);
+        assertThat(dto.getThumbnailUrl()).isNotEqualTo(THUMB_KEY);
+        assertThat(dto.getFileUrl()).startsWith("https://").contains("X-Amz-Signature=");
+        assertThat(dto.getThumbnailUrl()).startsWith("https://").contains("X-Amz-Signature=");
+        assertThat(dto.getUrlExpiresAt()).isAfter(Instant.now());
+    }
+
+    @Test
+    void thumbnailFallsBackToTheFullSizeUrlWhenAbsent() {
+        Upload upload = anonUpload(true);
+        upload.setThumbnailUrl(null);
+
+        UploadDto dto = service.toUploadDto(upload, new UploadService.UploadViewer(strangerId, false));
+
+        assertThat(dto.getThumbnailUrl()).isEqualTo(dto.getFileUrl());
+    }
+
+    @Test
+    void unapprovedContentYieldsNoUrlsForUnprivilegedCallersInsteadOfFailing() {
+        UploadDto dto = service.toUploadDto(anonUpload(false),
+                new UploadService.UploadViewer(strangerId, false));
+
+        assertThat(dto.getFileUrl()).isNull();
+        assertThat(dto.getThumbnailUrl()).isNull();
+        assertThat(dto.getUrlExpiresAt()).isNull();
+        assertThat(dto.getUuid()).isNotNull();
+    }
+
+    @Test
+    void unapprovedContentIsViewableByItsOwnUploader() {
+        UploadDto dto = service.toUploadDto(anonUpload(false),
+                new UploadService.UploadViewer(uploaderId, false));
+
+        assertThat(dto.getFileUrl()).contains("X-Amz-Signature=");
+    }
+
+    @Test
+    void unapprovedContentIsViewableByModerators() {
+        UploadDto dto = service.toUploadDto(anonUpload(false),
+                new UploadService.UploadViewer(strangerId, true));
+
+        assertThat(dto.getFileUrl()).contains("X-Amz-Signature=");
+    }
+
+    // ── Anonymity ─────────────────────────────────────────────────────────────
+
+    @Test
+    void anonymousUploadHidesUploaderAndHandleFromOtherUsers() {
+        UploadDto dto = service.toUploadDto(anonUpload(true),
+                new UploadService.UploadViewer(strangerId, false));
+
+        assertThat(dto.isAnon()).isTrue();
+        assertThat(dto.getUploadedBy()).isNull();
+        assertThat(dto.getInstagramHandle()).isNull();
+    }
+
+    @Test
+    void anonymousUploadHidesUploaderFromUnauthenticatedCallers() {
+        UploadDto dto = service.toUploadDto(anonUpload(true), UploadService.UploadViewer.anonymous());
+
+        assertThat(dto.getUploadedBy()).isNull();
+        assertThat(dto.getInstagramHandle()).isNull();
+    }
+
+    @Test
+    void anonymousUploadIsUnmaskedForTheUploaderThemselves() {
+        UploadDto dto = service.toUploadDto(anonUpload(true),
+                new UploadService.UploadViewer(uploaderId, false));
+
+        assertThat(dto.getUploadedBy()).isEqualTo(uploaderId);
+        assertThat(dto.getInstagramHandle()).isEqualTo("@secret_handle");
+    }
+
+    @Test
+    void anonymousUploadIsUnmaskedForHoldersOfMediaUploadRead() {
+        UploadDto dto = service.toUploadDto(anonUpload(true),
+                new UploadService.UploadViewer(strangerId, true));
+
+        assertThat(dto.getUploadedBy()).isEqualTo(uploaderId);
+        assertThat(dto.getInstagramHandle()).isEqualTo("@secret_handle");
+    }
+
+    @Test
+    void nonAnonymousUploadKeepsItsAttribution() {
+        Upload upload = anonUpload(true);
+        upload.setAnon(false);
+
+        UploadDto dto = service.toUploadDto(upload, new UploadService.UploadViewer(strangerId, false));
+
+        assertThat(dto.getUploadedBy()).isEqualTo(uploaderId);
+        assertThat(dto.getInstagramHandle()).isEqualTo("@secret_handle");
+    }
+
+    // ── Viewer resolution ─────────────────────────────────────────────────────
+
+    @Test
+    void viewerIsAnonymousWithNoAuthentication() {
+        SecurityContextHolder.clearContext();
+
+        UploadService.UploadViewer viewer = service.currentViewer();
+
+        assertThat(viewer.userId()).isNull();
+        assertThat(viewer.canReadAllUploads()).isFalse();
+    }
+
+    @Test
+    void viewerIsAnonymousForTheAnonymousToken() {
+        SecurityContextHolder.getContext().setAuthentication(
+                new AnonymousAuthenticationToken("key", "anonymousUser",
+                        List.of(new SimpleGrantedAuthority("ROLE_ANONYMOUS"))));
+
+        assertThat(service.currentViewer().userId()).isNull();
+    }
+
+    @Test
+    void viewerResolvesIdAndModeratorAuthority() {
+        when(userService.findByEmail("mod@example.com"))
+                .thenReturn(Optional.of(user(strangerId, "mod@example.com")));
+        authenticateAs("mod@example.com", Permission.Authority.MEDIA_UPLOAD_READ);
+
+        UploadService.UploadViewer viewer = service.currentViewer();
+
+        assertThat(viewer.userId()).isEqualTo(strangerId);
+        assertThat(viewer.canReadAllUploads()).isTrue();
+    }
+
+    @Test
+    void plainUserIsNotAMediaModerator() {
+        when(userService.findByEmail("user@example.com"))
+                .thenReturn(Optional.of(user(uploaderId, "user@example.com")));
+        authenticateAs("user@example.com", "ROLE_USER", Permission.Authority.MEDIA_UPLOAD_SELF);
+
+        UploadService.UploadViewer viewer = service.currentViewer();
+
+        assertThat(viewer.userId()).isEqualTo(uploaderId);
+        assertThat(viewer.canReadAllUploads()).isFalse();
+    }
+
+    @Test
+    void unresolvablePrincipalDegradesToLeastPrivilege() {
+        when(userService.findByEmail("ghost@example.com")).thenReturn(Optional.empty());
+        authenticateAs("ghost@example.com", "ROLE_USER");
+
+        UploadService.UploadViewer viewer = service.currentViewer();
+
+        assertThat(viewer.userId()).isNull();
+        assertThat(viewer.canReadAllUploads()).isFalse();
+        // …and a null-id viewer must not accidentally match a null-uploader upload.
+        Upload orphan = anonUpload(true);
+        orphan.setUploadedBy(null);
+        assertThat(viewer.isUploader(orphan)).isFalse();
+        assertThat(service.toUploadDto(orphan, viewer).getUploadedBy()).isNull();
+    }
+}