From 053042f127e8393cacb293b10e734543ae8607ea Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 15 Aug 2026 21:15:29 +0900 Subject: [PATCH 1/8] fix(security): add tenant admin permission on clean main --- .../java/com/clearfolio/viewer/auth/TenantPermissions.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/main/java/com/clearfolio/viewer/auth/TenantPermissions.java b/src/main/java/com/clearfolio/viewer/auth/TenantPermissions.java index 4f9c6a685..73befa765 100644 --- a/src/main/java/com/clearfolio/viewer/auth/TenantPermissions.java +++ b/src/main/java/com/clearfolio/viewer/auth/TenantPermissions.java @@ -55,6 +55,11 @@ public final class TenantPermissions { */ public static final String ANALYTICS_READ = "analytics:read"; + /** + * Permission required to configure tenant or access admin endpoints. + */ + public static final String TENANT_CONFIGURE = "tenant:configure"; + private TenantPermissions() { } } From 0de81396a69ffbbd23be30cea251e14ceb501980 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 15 Aug 2026 21:17:59 +0900 Subject: [PATCH 2/8] fix(security): scope admin operations by tenant --- .../viewer/controller/AdminController.java | 72 ++++++++++++++----- 1 file changed, 56 insertions(+), 16 deletions(-) diff --git a/src/main/java/com/clearfolio/viewer/controller/AdminController.java b/src/main/java/com/clearfolio/viewer/controller/AdminController.java index 412d4eb86..cbe80fa43 100644 --- a/src/main/java/com/clearfolio/viewer/controller/AdminController.java +++ b/src/main/java/com/clearfolio/viewer/controller/AdminController.java @@ -4,47 +4,66 @@ import java.util.List; import java.util.UUID; +import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.DeleteMapping; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.RequestHeader; import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.RestController; import org.springframework.web.server.ResponseStatusException; import com.clearfolio.viewer.api.AdminJobListResponse; +import com.clearfolio.viewer.auth.TenantAccessService; +import com.clearfolio.viewer.auth.TenantContext; +import com.clearfolio.viewer.auth.TenantPermissions; import com.clearfolio.viewer.model.ConversionJob; import com.clearfolio.viewer.service.DocumentConversionService; import com.clearfolio.viewer.service.RetryDeadLetterResult; /** - * Controller for admin-specific endpoints. + * Controller for tenant-scoped administrative conversion operations. */ @RestController public class AdminController { + /** Conversion service. */ private final DocumentConversionService conversionService; + /** Tenant access service. */ + private final TenantAccessService tenantAccessService; + /** * Creates a controller for admin operations. * - * @param conversionService conversion service + * @param conversionSvc conversion service + * @param tenantAccessSvc tenant access service */ - public AdminController(DocumentConversionService conversionService) { - this.conversionService = conversionService; + public AdminController(final DocumentConversionService conversionSvc, + final TenantAccessService tenantAccessSvc) { + this.conversionService = conversionSvc; + this.tenantAccessService = tenantAccessSvc; } /** - * Retrieves all conversion jobs, optionally filtered by dead-letter status. + * Retrieves tenant-owned conversion jobs, optionally filtered by dead-letter status. * * @param deadLettered optional filter for dead-lettered jobs - * @return list of conversion jobs + * @param headers authenticated tenant claim headers + * @return tenant-scoped list of conversion jobs */ @GetMapping("/api/v1/admin/convert/jobs") - public AdminJobListResponse getAllJobs(@RequestParam(required = false) Boolean deadLettered) { - Iterable allJobs = conversionService.getAllJobs(); + public AdminJobListResponse getAllJobs( + @RequestParam(required = false) final Boolean deadLettered, + @RequestHeader final HttpHeaders headers) { + TenantContext tenantContext = tenantAccessService.require( + headers, + TenantPermissions.TENANT_CONFIGURE + ); + Iterable allJobs = conversionService.getAllJobs(tenantContext); if (deadLettered == null) { return AdminJobListResponse.from(allJobs); @@ -60,31 +79,52 @@ public AdminJobListResponse getAllJobs(@RequestParam(required = false) Boolean d } /** - * Deletes a conversion job. + * Deletes a conversion job owned by the authenticated tenant. * * @param jobId conversion job identifier + * @param headers authenticated tenant claim headers * @return no content on success */ @DeleteMapping("/api/v1/admin/convert/jobs/{jobId}") - public ResponseEntity deleteJob(@PathVariable UUID jobId) { - conversionService.deleteJob(jobId); + public ResponseEntity deleteJob(@PathVariable final UUID jobId, + @RequestHeader final HttpHeaders headers) { + TenantContext tenantContext = tenantAccessService.require( + headers, + TenantPermissions.TENANT_CONFIGURE + ); + if (!conversionService.deleteJob(jobId, tenantContext)) { + throw new ResponseStatusException(HttpStatus.NOT_FOUND, "job not found"); + } return ResponseEntity.noContent().build(); } /** - * Retries a dead-lettered conversion job. + * Retries a dead-lettered conversion job owned by the authenticated tenant. * * @param jobId conversion job identifier + * @param headers authenticated tenant claim headers * @return accepted response on success */ @PostMapping("/api/v1/admin/convert/jobs/{jobId}/retry") - public ResponseEntity retryDeadLettered(@PathVariable UUID jobId) { - RetryDeadLetterResult result = conversionService.retryDeadLettered(jobId, "admin"); + public ResponseEntity retryDeadLettered( + @PathVariable final UUID jobId, + @RequestHeader final HttpHeaders headers) { + TenantContext tenantContext = tenantAccessService.require( + headers, + TenantPermissions.TENANT_CONFIGURE + ); + RetryDeadLetterResult result = conversionService.retryDeadLettered( + jobId, + "admin", + tenantContext + ); if (result == RetryDeadLetterResult.NOT_FOUND) { - throw new ResponseStatusException(HttpStatus.NOT_FOUND, "job not found"); + throw new ResponseStatusException( + HttpStatus.NOT_FOUND, "job not found"); } if (result == RetryDeadLetterResult.NOT_ELIGIBLE) { - throw new ResponseStatusException(HttpStatus.CONFLICT, "job is not eligible for retry"); + throw new ResponseStatusException( + HttpStatus.CONFLICT, "job is not eligible for retry"); } return ResponseEntity.accepted().build(); } From ae63bf43c5a3eedbe22662bf678d56c5f3c999ae Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 15 Aug 2026 21:18:31 +0900 Subject: [PATCH 3/8] fix(security): add tenant-scoped service defaults --- .../service/DocumentConversionService.java | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/src/main/java/com/clearfolio/viewer/service/DocumentConversionService.java b/src/main/java/com/clearfolio/viewer/service/DocumentConversionService.java index 6676f5610..db4575a42 100644 --- a/src/main/java/com/clearfolio/viewer/service/DocumentConversionService.java +++ b/src/main/java/com/clearfolio/viewer/service/DocumentConversionService.java @@ -1,5 +1,7 @@ package com.clearfolio.viewer.service; +import java.util.ArrayList; +import java.util.List; import java.util.Optional; import java.util.UUID; @@ -60,6 +62,36 @@ default UUID submit(MultipartFile file, PolicyOverrideRequest overrideRequest, T */ RetryDeadLetterResult retryDeadLettered(UUID jobId, String operatorId); + /** + * Retries only a dead-lettered job owned by the supplied tenant. + * + *

The compatibility default checks ownership before delegating to the + * legacy mutation. Durable implementations should override this method with + * one storage-scoped, generation-fenced transition.

+ * + * @param jobId conversion job identifier + * @param operatorId operator identifier that triggered the retry + * @param tenantContext authenticated tenant authority + * @return retry outcome, with missing and foreign jobs both concealed as + * {@link RetryDeadLetterResult#NOT_FOUND} + */ + default RetryDeadLetterResult retryDeadLettered( + UUID jobId, + String operatorId, + TenantContext tenantContext + ) { + if (tenantContext == null) { + return RetryDeadLetterResult.NOT_FOUND; + } + + Optional job = getJob(jobId); + if (job.isEmpty() || !job.get().belongsToTenant(tenantContext.tenantId())) { + return RetryDeadLetterResult.NOT_FOUND; + } + + return retryDeadLettered(jobId, operatorId); + } + /** * Deletes a conversion job owned by the supplied tenant context. * @@ -95,4 +127,30 @@ default boolean deleteJob(UUID jobId, TenantContext tenantContext) { * @return an iterable of all conversion jobs */ Iterable getAllJobs(); + + /** + * Returns only jobs owned by the supplied authenticated tenant. + * + *

The compatibility default prevents cross-tenant publication even when + * an older adapter exposes only a global inventory. Durable implementations + * should override this method with a tenant predicate at the storage query + * boundary.

+ * + * @param tenantContext authenticated tenant authority + * @return immutable snapshot of tenant-owned jobs, or an empty result when + * tenant authority is absent + */ + default Iterable getAllJobs(TenantContext tenantContext) { + if (tenantContext == null) { + return List.of(); + } + + List tenantJobs = new ArrayList<>(); + for (ConversionJob job : getAllJobs()) { + if (job.belongsToTenant(tenantContext.tenantId())) { + tenantJobs.add(job); + } + } + return List.copyOf(tenantJobs); + } } From 80fd7098431256b5095fc57be1f424d90386f3c6 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 15 Aug 2026 21:19:54 +0900 Subject: [PATCH 4/8] test(security): cover tenant-scoped admin authorization --- .../controller/AdminControllerTest.java | 218 +++++++++++++++--- 1 file changed, 187 insertions(+), 31 deletions(-) diff --git a/src/test/java/com/clearfolio/viewer/controller/AdminControllerTest.java b/src/test/java/com/clearfolio/viewer/controller/AdminControllerTest.java index ad63a8015..d36d55ce8 100644 --- a/src/test/java/com/clearfolio/viewer/controller/AdminControllerTest.java +++ b/src/test/java/com/clearfolio/viewer/controller/AdminControllerTest.java @@ -1,124 +1,280 @@ package com.clearfolio.viewer.controller; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import java.util.Arrays; +import java.util.Set; import java.util.UUID; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.springframework.http.HttpHeaders; import org.springframework.test.web.reactive.server.WebTestClient; +import com.clearfolio.viewer.auth.TenantAccessService; +import com.clearfolio.viewer.auth.TenantContext; +import com.clearfolio.viewer.auth.TenantPermissions; import com.clearfolio.viewer.model.ConversionJob; import com.clearfolio.viewer.service.DocumentConversionService; import com.clearfolio.viewer.service.RetryDeadLetterResult; class AdminControllerTest { + private static final String TENANT_ID = "tenant-a"; + private static final String SUBJECT_ID = "operator-a"; + private DocumentConversionService conversionService; private WebTestClient webTestClient; - private AdminController controller; @BeforeEach void setUp() { conversionService = mock(DocumentConversionService.class); - controller = new AdminController(conversionService); + AdminController controller = new AdminController( + conversionService, + new TenantAccessService() + ); webTestClient = WebTestClient.bindToController(controller) .controllerAdvice(new ApiExceptionHandler()) .build(); } @Test - void getAllJobsReturnsAllJobsWhenNoFilterProvided() { - ConversionJob job1 = new ConversionJob(UUID.randomUUID(), "a.pdf", "application/pdf", "hash-a", 100L); - ConversionJob job2 = new ConversionJob(UUID.randomUUID(), "b.pdf", "application/pdf", "hash-b", 100L); - when(conversionService.getAllJobs()).thenReturn(Arrays.asList(job1, job2)); + void getAllJobsReturnsTenantScopedJobsWhenNoFilterProvided() { + TenantContext context = tenantContext(); + ConversionJob job1 = tenantJob("a.pdf", "hash-a"); + ConversionJob job2 = tenantJob("b.pdf", "hash-b"); + when(conversionService.getAllJobs(eq(context))).thenReturn(Arrays.asList(job1, job2)); webTestClient.get() .uri("/api/v1/admin/convert/jobs") + .headers(AdminControllerTest::addAdminHeaders) .exchange() .expectStatus().isOk() .expectBody() .jsonPath("$.jobs.length()").isEqualTo(2) .jsonPath("$.jobs[0].fileName").isEqualTo("a.pdf") .jsonPath("$.jobs[1].fileName").isEqualTo("b.pdf"); + + verify(conversionService).getAllJobs(eq(context)); + verify(conversionService, never()).getAllJobs(); } @Test - void getAllJobsFiltersByDeadLetteredTrue() { - ConversionJob job1 = new ConversionJob(UUID.randomUUID(), "a.pdf", "application/pdf", "hash-a", 100L); - job1.markDeadLettered("failed"); - ConversionJob job2 = new ConversionJob(UUID.randomUUID(), "b.pdf", "application/pdf", "hash-b", 100L); - - when(conversionService.getAllJobs()).thenReturn(Arrays.asList(job1, job2)); + void getAllJobsFiltersTenantScopedDeadLetteredJobs() { + TenantContext context = tenantContext(); + ConversionJob deadLetteredJob = tenantJob("dead.pdf", "hash-dead"); + deadLetteredJob.markDeadLettered("failed"); + ConversionJob activeJob = tenantJob("active.pdf", "hash-active"); + when(conversionService.getAllJobs(eq(context))).thenReturn( + Arrays.asList(deadLetteredJob, activeJob) + ); webTestClient.get() .uri("/api/v1/admin/convert/jobs?deadLettered=true") + .headers(AdminControllerTest::addAdminHeaders) .exchange() .expectStatus().isOk() .expectBody() .jsonPath("$.jobs.length()").isEqualTo(1) - .jsonPath("$.jobs[0].fileName").isEqualTo("a.pdf"); - } - - @Test - void getAllJobsFiltersByDeadLetteredFalse() { - ConversionJob job1 = new ConversionJob(UUID.randomUUID(), "a.pdf", "application/pdf", "hash-a", 100L); - job1.markDeadLettered("failed"); - ConversionJob job2 = new ConversionJob(UUID.randomUUID(), "b.pdf", "application/pdf", "hash-b", 100L); - - when(conversionService.getAllJobs()).thenReturn(Arrays.asList(job1, job2)); + .jsonPath("$.jobs[0].fileName").isEqualTo("dead.pdf"); webTestClient.get() .uri("/api/v1/admin/convert/jobs?deadLettered=false") + .headers(AdminControllerTest::addAdminHeaders) .exchange() .expectStatus().isOk() .expectBody() .jsonPath("$.jobs.length()").isEqualTo(1) - .jsonPath("$.jobs[0].fileName").isEqualTo("b.pdf"); + .jsonPath("$.jobs[0].fileName").isEqualTo("active.pdf"); + + verify(conversionService, org.mockito.Mockito.times(2)).getAllJobs(eq(context)); + verify(conversionService, never()).getAllJobs(); } @Test - void deleteJobReturnsNoContent() { + void deleteJobUsesTenantScopedMutationAndReturnsNoContent() { UUID jobId = UUID.randomUUID(); + TenantContext context = tenantContext(); + when(conversionService.deleteJob(eq(jobId), eq(context))).thenReturn(true); webTestClient.delete() .uri("/api/v1/admin/convert/jobs/" + jobId) + .headers(AdminControllerTest::addAdminHeaders) .exchange() .expectStatus().isNoContent(); + + verify(conversionService).deleteJob(eq(jobId), eq(context)); + verify(conversionService, never()).deleteJob(jobId); + } + + @Test + void deleteJobConcealsMissingOrForeignJobsAsNotFound() { + UUID jobId = UUID.randomUUID(); + TenantContext context = tenantContext(); + when(conversionService.deleteJob(eq(jobId), eq(context))).thenReturn(false); + + webTestClient.delete() + .uri("/api/v1/admin/convert/jobs/" + jobId) + .headers(AdminControllerTest::addAdminHeaders) + .exchange() + .expectStatus().isNotFound(); + + verify(conversionService).deleteJob(eq(jobId), eq(context)); + verify(conversionService, never()).deleteJob(jobId); } @Test - void retryDeadLetteredReturnsAcceptedWhenAccepted() { + void retryDeadLetteredUsesTenantScopedMutationWhenAccepted() { UUID jobId = UUID.randomUUID(); - when(conversionService.retryDeadLettered(jobId, "admin")).thenReturn(RetryDeadLetterResult.ACCEPTED); + TenantContext context = tenantContext(); + when(conversionService.retryDeadLettered(jobId, "admin", context)) + .thenReturn(RetryDeadLetterResult.ACCEPTED); webTestClient.post() .uri("/api/v1/admin/convert/jobs/" + jobId + "/retry") + .headers(AdminControllerTest::addAdminHeaders) .exchange() .expectStatus().isAccepted(); + + verify(conversionService).retryDeadLettered(jobId, "admin", context); + verify(conversionService, never()).retryDeadLettered(jobId, "admin"); } @Test - void retryDeadLetteredReturnsNotFoundWhenNotFound() { + void retryDeadLetteredConcealsMissingOrForeignJobsAsNotFound() { UUID jobId = UUID.randomUUID(); - when(conversionService.retryDeadLettered(jobId, "admin")).thenReturn(RetryDeadLetterResult.NOT_FOUND); + TenantContext context = tenantContext(); + when(conversionService.retryDeadLettered(jobId, "admin", context)) + .thenReturn(RetryDeadLetterResult.NOT_FOUND); webTestClient.post() .uri("/api/v1/admin/convert/jobs/" + jobId + "/retry") + .headers(AdminControllerTest::addAdminHeaders) .exchange() .expectStatus().isNotFound(); + + verify(conversionService).retryDeadLettered(jobId, "admin", context); + verify(conversionService, never()).retryDeadLettered(jobId, "admin"); + } + + @Test + void retryDeadLetteredReturnsConflictWhenOwnedJobIsNotEligible() { + UUID jobId = UUID.randomUUID(); + TenantContext context = tenantContext(); + when(conversionService.retryDeadLettered(jobId, "admin", context)) + .thenReturn(RetryDeadLetterResult.NOT_ELIGIBLE); + + webTestClient.post() + .uri("/api/v1/admin/convert/jobs/" + jobId + "/retry") + .headers(AdminControllerTest::addAdminHeaders) + .exchange() + .expectStatus().isEqualTo(409); + + verify(conversionService).retryDeadLettered(jobId, "admin", context); + verify(conversionService, never()).retryDeadLettered(jobId, "admin"); } @Test - void retryDeadLetteredReturnsConflictWhenNotEligible() { + void everyAdminEndpointRejectsMissingClaimsBeforeServiceAccess() { UUID jobId = UUID.randomUUID(); - when(conversionService.retryDeadLettered(jobId, "admin")).thenReturn(RetryDeadLetterResult.NOT_ELIGIBLE); + webTestClient.get() + .uri("/api/v1/admin/convert/jobs") + .exchange() + .expectStatus().isUnauthorized(); + webTestClient.delete() + .uri("/api/v1/admin/convert/jobs/" + jobId) + .exchange() + .expectStatus().isUnauthorized(); webTestClient.post() .uri("/api/v1/admin/convert/jobs/" + jobId + "/retry") .exchange() - .expectStatus().isEqualTo(409); // isConflict() isn't always available depending on spring-test version, so using isEqualTo(409) is safer + .expectStatus().isUnauthorized(); + + verifyNoInteractions(conversionService); + } + + @Test + void everyAdminEndpointRequiresTenantConfigurePermission() { + UUID jobId = UUID.randomUUID(); + + webTestClient.get() + .uri("/api/v1/admin/convert/jobs") + .headers(headers -> addHeaders(headers, TenantPermissions.JOB_READ)) + .exchange() + .expectStatus().isForbidden(); + webTestClient.delete() + .uri("/api/v1/admin/convert/jobs/" + jobId) + .headers(headers -> addHeaders(headers, TenantPermissions.JOB_DELETE)) + .exchange() + .expectStatus().isForbidden(); + webTestClient.post() + .uri("/api/v1/admin/convert/jobs/" + jobId + "/retry") + .headers(headers -> addHeaders(headers, TenantPermissions.JOB_RETRY)) + .exchange() + .expectStatus().isForbidden(); + + verifyNoInteractions(conversionService); + } + + @Test + void blankTenantOrSubjectClaimsFailClosedBeforeServiceAccess() { + webTestClient.get() + .uri("/api/v1/admin/convert/jobs") + .headers(headers -> { + headers.set(TenantContext.TENANT_ID_HEADER, " "); + headers.set(TenantContext.SUBJECT_ID_HEADER, SUBJECT_ID); + headers.set(TenantContext.PERMISSIONS_HEADER, TenantPermissions.TENANT_CONFIGURE); + }) + .exchange() + .expectStatus().isUnauthorized(); + + webTestClient.get() + .uri("/api/v1/admin/convert/jobs") + .headers(headers -> { + headers.set(TenantContext.TENANT_ID_HEADER, TENANT_ID); + headers.set(TenantContext.SUBJECT_ID_HEADER, " "); + headers.set(TenantContext.PERMISSIONS_HEADER, TenantPermissions.TENANT_CONFIGURE); + }) + .exchange() + .expectStatus().isUnauthorized(); + + verifyNoInteractions(conversionService); + } + + private static ConversionJob tenantJob(String fileName, String contentHash) { + return new ConversionJob( + UUID.randomUUID(), + TENANT_ID, + SUBJECT_ID, + fileName, + "application/pdf", + contentHash, + 100L, + 3 + ); + } + + private static TenantContext tenantContext() { + return new TenantContext( + TENANT_ID, + SUBJECT_ID, + Set.of(TenantPermissions.TENANT_CONFIGURE) + ); + } + + private static void addAdminHeaders(HttpHeaders headers) { + addHeaders(headers, TenantPermissions.TENANT_CONFIGURE); + } + + private static void addHeaders(HttpHeaders headers, String permission) { + headers.set(TenantContext.TENANT_ID_HEADER, TENANT_ID); + headers.set(TenantContext.SUBJECT_ID_HEADER, SUBJECT_ID); + headers.set(TenantContext.PERMISSIONS_HEADER, permission); } } From da48e22c9a2095092f1f0def55bba46a2d79ed4d Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 15 Aug 2026 21:20:48 +0900 Subject: [PATCH 5/8] docs(security): record tenant-scoped admin control --- .jules/sentinel.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index e795cb9dd..0d33cb29e 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -32,3 +32,8 @@ **Vulnerability:** The document hashing routine in `DefaultDocumentConversionService` processed file streams without enforcing any maximum size limit on the bytes read. An attacker could exploit this by uploading a maliciously large stream (or exploiting a compression bomb if unzipping), exhausting system memory, CPU, or disk space (DoS). **Learning:** Checking the declared file size (e.g., `file.getSize()`) in initial validation is not always sufficient if the input stream itself can be spoofed or dynamically expanded during reading. The actual bytes read must be verified against bounds continuously. **Prevention:** Always enforce a strict, configurable size limit (e.g., `ConversionProperties.maxUploadSizeBytes`) within the `while` loop that reads from untrusted input streams. Track `totalRead` and throw an exception immediately if the limit is exceeded. + +## 2026-08-13 - [CRITICAL] Admin API 인증 및 권한 검증 누락 수정 +**Vulnerability:** `AdminController`의 관리자 전용 API(전체 작업 조회, 작업 삭제, 데드레터 재시도 등)에 대해 사용자 인증 및 권한(tenant:configure) 검증이 누락되어 누구나 관리자 기능을 호출할 수 있었습니다. +**Learning:** 새로운 컨트롤러 추가 시 기존의 `TenantAccessService`를 통한 AOP/필터 기반 공통 처리가 아닌, 각 메서드 내에서 명시적인 권한 요구(tenantAccessService.require) 방식을 사용하고 있어 휴먼 에러로 검증 코드가 쉽게 누락될 수 있었습니다. +**Prevention:** 모든 엔드포인트는 추가 시 반드시 권한 검증 로직이 포함되었는지 확인하고, 보안 문서에 정의된 권한(`docs/security/2026-07-02-auth-tenant-model.md`의 `tenant_admin` 권한)과 일치하는 상수(예: `TENANT_CONFIGURE`)를 적용해야 합니다. From f965ce89fa795fde216cc93c412fe5fc1b7c034b Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 15 Aug 2026 22:26:43 +0900 Subject: [PATCH 6/8] test(security): bind admin retries to authenticated subject --- .../viewer/controller/AdminControllerTest.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/test/java/com/clearfolio/viewer/controller/AdminControllerTest.java b/src/test/java/com/clearfolio/viewer/controller/AdminControllerTest.java index d36d55ce8..7dea4242c 100644 --- a/src/test/java/com/clearfolio/viewer/controller/AdminControllerTest.java +++ b/src/test/java/com/clearfolio/viewer/controller/AdminControllerTest.java @@ -132,7 +132,7 @@ void deleteJobConcealsMissingOrForeignJobsAsNotFound() { void retryDeadLetteredUsesTenantScopedMutationWhenAccepted() { UUID jobId = UUID.randomUUID(); TenantContext context = tenantContext(); - when(conversionService.retryDeadLettered(jobId, "admin", context)) + when(conversionService.retryDeadLettered(jobId, SUBJECT_ID, context)) .thenReturn(RetryDeadLetterResult.ACCEPTED); webTestClient.post() @@ -141,15 +141,15 @@ void retryDeadLetteredUsesTenantScopedMutationWhenAccepted() { .exchange() .expectStatus().isAccepted(); - verify(conversionService).retryDeadLettered(jobId, "admin", context); - verify(conversionService, never()).retryDeadLettered(jobId, "admin"); + verify(conversionService).retryDeadLettered(jobId, SUBJECT_ID, context); + verify(conversionService, never()).retryDeadLettered(jobId, SUBJECT_ID); } @Test void retryDeadLetteredConcealsMissingOrForeignJobsAsNotFound() { UUID jobId = UUID.randomUUID(); TenantContext context = tenantContext(); - when(conversionService.retryDeadLettered(jobId, "admin", context)) + when(conversionService.retryDeadLettered(jobId, SUBJECT_ID, context)) .thenReturn(RetryDeadLetterResult.NOT_FOUND); webTestClient.post() @@ -158,15 +158,15 @@ void retryDeadLetteredConcealsMissingOrForeignJobsAsNotFound() { .exchange() .expectStatus().isNotFound(); - verify(conversionService).retryDeadLettered(jobId, "admin", context); - verify(conversionService, never()).retryDeadLettered(jobId, "admin"); + verify(conversionService).retryDeadLettered(jobId, SUBJECT_ID, context); + verify(conversionService, never()).retryDeadLettered(jobId, SUBJECT_ID); } @Test void retryDeadLetteredReturnsConflictWhenOwnedJobIsNotEligible() { UUID jobId = UUID.randomUUID(); TenantContext context = tenantContext(); - when(conversionService.retryDeadLettered(jobId, "admin", context)) + when(conversionService.retryDeadLettered(jobId, SUBJECT_ID, context)) .thenReturn(RetryDeadLetterResult.NOT_ELIGIBLE); webTestClient.post() @@ -175,8 +175,8 @@ void retryDeadLetteredReturnsConflictWhenOwnedJobIsNotEligible() { .exchange() .expectStatus().isEqualTo(409); - verify(conversionService).retryDeadLettered(jobId, "admin", context); - verify(conversionService, never()).retryDeadLettered(jobId, "admin"); + verify(conversionService).retryDeadLettered(jobId, SUBJECT_ID, context); + verify(conversionService, never()).retryDeadLettered(jobId, SUBJECT_ID); } @Test From dbfb79556212ac073aeca9e805126274f5c63a7f Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 15 Aug 2026 22:27:19 +0900 Subject: [PATCH 7/8] fix(security): audit retries as the authenticated subject --- .../java/com/clearfolio/viewer/controller/AdminController.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/clearfolio/viewer/controller/AdminController.java b/src/main/java/com/clearfolio/viewer/controller/AdminController.java index cbe80fa43..4d609f3f1 100644 --- a/src/main/java/com/clearfolio/viewer/controller/AdminController.java +++ b/src/main/java/com/clearfolio/viewer/controller/AdminController.java @@ -115,7 +115,7 @@ public ResponseEntity retryDeadLettered( ); RetryDeadLetterResult result = conversionService.retryDeadLettered( jobId, - "admin", + tenantContext.subjectId(), tenantContext ); if (result == RetryDeadLetterResult.NOT_FOUND) { From 3826edb6a6eaa5697c666cc5babd052b35604635 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 17 Aug 2026 08:03:40 +0900 Subject: [PATCH 8/8] test(security): cover tenant-scope compatibility defaults --- ...umentConversionServiceTenantScopeTest.java | 178 ++++++++++++++++++ 1 file changed, 178 insertions(+) create mode 100644 src/test/java/com/clearfolio/viewer/service/DocumentConversionServiceTenantScopeTest.java diff --git a/src/test/java/com/clearfolio/viewer/service/DocumentConversionServiceTenantScopeTest.java b/src/test/java/com/clearfolio/viewer/service/DocumentConversionServiceTenantScopeTest.java new file mode 100644 index 000000000..97dfc5b85 --- /dev/null +++ b/src/test/java/com/clearfolio/viewer/service/DocumentConversionServiceTenantScopeTest.java @@ -0,0 +1,178 @@ +package com.clearfolio.viewer.service; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; +import java.util.Set; +import java.util.UUID; + +import org.junit.jupiter.api.Test; +import org.springframework.web.multipart.MultipartFile; + +import com.clearfolio.viewer.auth.TenantContext; +import com.clearfolio.viewer.model.ConversionJob; + +/** + * Regression coverage for the legacy-service tenant-scope compatibility defaults. + */ +class DocumentConversionServiceTenantScopeTest { + + private static final String TENANT_ID = "tenant-a"; + private static final String SUBJECT_ID = "operator-a"; + + /** + * Missing tenant authority must fail closed before a job lookup or mutation. + */ + @Test + void retryRejectsMissingTenantAuthorityBeforeLookup() { + LegacyService service = new LegacyService(List.of(ownedJob())); + UUID jobId = service.jobs.getFirst().getJobId(); + + RetryDeadLetterResult result = service.retryDeadLettered(jobId, SUBJECT_ID, null); + + assertThat(result).isEqualTo(RetryDeadLetterResult.NOT_FOUND); + assertThat(service.getJobCalls).isZero(); + assertThat(service.legacyRetryCalls).isZero(); + } + + /** + * Missing and foreign jobs must be concealed without invoking the legacy mutation. + */ + @Test + void retryConcealsMissingAndForeignJobs() { + LegacyService missingService = new LegacyService(List.of()); + UUID missingJobId = UUID.randomUUID(); + + assertThat(missingService.retryDeadLettered(missingJobId, SUBJECT_ID, tenantContext())) + .isEqualTo(RetryDeadLetterResult.NOT_FOUND); + assertThat(missingService.legacyRetryCalls).isZero(); + + ConversionJob foreignJob = job("tenant-b", "foreign.pdf"); + LegacyService foreignService = new LegacyService(List.of(foreignJob)); + + assertThat(foreignService.retryDeadLettered( + foreignJob.getJobId(), + SUBJECT_ID, + tenantContext() + )).isEqualTo(RetryDeadLetterResult.NOT_FOUND); + assertThat(foreignService.legacyRetryCalls).isZero(); + } + + /** + * An owned job may delegate to the legacy retry using the authenticated operator identity. + */ + @Test + void retryDelegatesOwnedJobWithAuthenticatedOperator() { + ConversionJob ownedJob = ownedJob(); + LegacyService service = new LegacyService(List.of(ownedJob)); + + RetryDeadLetterResult result = service.retryDeadLettered( + ownedJob.getJobId(), + SUBJECT_ID, + tenantContext() + ); + + assertThat(result).isEqualTo(RetryDeadLetterResult.ACCEPTED); + assertThat(service.legacyRetryCalls).isEqualTo(1); + assertThat(service.lastRetriedJobId).isEqualTo(ownedJob.getJobId()); + assertThat(service.lastOperatorId).isEqualTo(SUBJECT_ID); + } + + /** + * Missing tenant authority must return an empty snapshot without reading the global inventory. + */ + @Test + void listRejectsMissingTenantAuthorityWithoutGlobalRead() { + LegacyService service = new LegacyService(List.of(ownedJob())); + + assertThat(service.getAllJobs(null)).isEmpty(); + assertThat(service.getAllJobsCalls).isZero(); + } + + /** + * The compatibility list filters the legacy global inventory at the tenant boundary. + */ + @Test + void listReturnsOnlyOwnedJobs() { + ConversionJob firstOwned = ownedJob(); + ConversionJob foreign = job("tenant-b", "foreign.pdf"); + ConversionJob secondOwned = job(TENANT_ID, "second.pdf"); + LegacyService service = new LegacyService(List.of(firstOwned, foreign, secondOwned)); + + assertThat(service.getAllJobs(tenantContext())) + .containsExactly(firstOwned, secondOwned); + assertThat(service.getAllJobsCalls).isEqualTo(1); + } + + private static TenantContext tenantContext() { + return new TenantContext(TENANT_ID, SUBJECT_ID, Set.of("tenant:configure")); + } + + private static ConversionJob ownedJob() { + return job(TENANT_ID, "owned.pdf"); + } + + private static ConversionJob job(String tenantId, String fileName) { + return new ConversionJob( + UUID.randomUUID(), + tenantId, + SUBJECT_ID, + fileName, + "application/pdf", + "hash-" + fileName, + 100L, + 3 + ); + } + + /** + * Minimal legacy implementation used to observe default-method delegation. + */ + private static final class LegacyService implements DocumentConversionService { + + private final List jobs; + private int getJobCalls; + private int getAllJobsCalls; + private int legacyRetryCalls; + private UUID lastRetriedJobId; + private String lastOperatorId; + + LegacyService(List jobs) { + this.jobs = new ArrayList<>(jobs); + } + + @Override + public UUID submit(MultipartFile file) { + throw new UnsupportedOperationException("submission is outside this test seam"); + } + + @Override + public Optional getJob(UUID jobId) { + getJobCalls += 1; + return jobs.stream() + .filter(job -> job.getJobId().equals(jobId)) + .findFirst(); + } + + @Override + public RetryDeadLetterResult retryDeadLettered(UUID jobId, String operatorId) { + legacyRetryCalls += 1; + lastRetriedJobId = jobId; + lastOperatorId = operatorId; + return RetryDeadLetterResult.ACCEPTED; + } + + @Override + public void deleteJob(UUID jobId) { + jobs.removeIf(job -> job.getJobId().equals(jobId)); + } + + @Override + public Iterable getAllJobs() { + getAllJobsCalls += 1; + return List.copyOf(jobs); + } + } +}