diff --git a/src/main/java/com/clearfolio/viewer/repository/ConversionJobRepository.java b/src/main/java/com/clearfolio/viewer/repository/ConversionJobRepository.java index b1cc9ae51..6ba4abde0 100644 --- a/src/main/java/com/clearfolio/viewer/repository/ConversionJobRepository.java +++ b/src/main/java/com/clearfolio/viewer/repository/ConversionJobRepository.java @@ -61,12 +61,18 @@ default Optional findByTenantAndContentHash(String tenantId, Stri /** * Finds a conversion job by tenant and identifier. * - * @param tenantId tenant identifier + *

The default deliberately fails closed rather than reading a globally + * addressable job and applying tenant ownership after retrieval. Durable + * adapters must override this method with a tenant predicate at the storage + * lookup boundary.

+ * + * @param tenantId authenticated tenant identifier * @param jobId conversion job identifier - * @return matching conversion job when found and owned by the tenant + * @return matching tenant-owned job, or empty until the adapter implements + * a storage-scoped lookup */ default Optional findByTenantAndId(String tenantId, UUID jobId) { - return findById(jobId).filter(job -> job.belongsToTenant(tenantId)); + return Optional.empty(); } /** diff --git a/src/main/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepository.java b/src/main/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepository.java index 03fd355bc..fb31378fd 100644 --- a/src/main/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepository.java +++ b/src/main/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepository.java @@ -122,6 +122,19 @@ public Optional findByTenantAndContentHash(String tenantId, Strin return findById(jobId); } + /** + * {@inheritDoc} + */ + @Override + public Optional findByTenantAndId(String tenantId, UUID jobId) { + if (tenantId == null || tenantId.isBlank() || jobId == null) { + return Optional.empty(); + } + String normalizedTenantId = tenantId.strip(); + return Optional.ofNullable(jobs.get(jobId)) + .filter(job -> job.belongsToTenant(normalizedTenantId)); + } + /** * {@inheritDoc} */ diff --git a/src/main/java/com/clearfolio/viewer/service/DefaultDocumentConversionService.java b/src/main/java/com/clearfolio/viewer/service/DefaultDocumentConversionService.java index 3ee96b9e8..f4d661a1d 100644 --- a/src/main/java/com/clearfolio/viewer/service/DefaultDocumentConversionService.java +++ b/src/main/java/com/clearfolio/viewer/service/DefaultDocumentConversionService.java @@ -213,6 +213,17 @@ public Optional getJob(UUID jobId) { return repository.findById(jobId); } + /** + * {@inheritDoc} + */ + @Override + public Optional getJob(UUID jobId, TenantContext tenantContext) { + if (jobId == null || tenantContext == null) { + return Optional.empty(); + } + return repository.findByTenantAndId(tenantContext.tenantId(), jobId); + } + /** * {@inheritDoc} */ diff --git a/src/main/java/com/clearfolio/viewer/service/DocumentConversionService.java b/src/main/java/com/clearfolio/viewer/service/DocumentConversionService.java index 6676f5610..6ebe09d7d 100644 --- a/src/main/java/com/clearfolio/viewer/service/DocumentConversionService.java +++ b/src/main/java/com/clearfolio/viewer/service/DocumentConversionService.java @@ -51,6 +51,22 @@ default UUID submit(MultipartFile file, PolicyOverrideRequest overrideRequest, T */ Optional getJob(UUID jobId); + /** + * Retrieves a conversion job through a tenant-scoped lookup boundary. + * + *

The default deliberately returns no result rather than performing a + * globally addressable lookup followed by an ownership filter. Implementations + * backed by tenant-aware storage should override this method and push the tenant + * predicate into that storage lookup.

+ * + * @param jobId conversion job identifier + * @param tenantContext authenticated tenant and subject authority + * @return matching tenant-owned job, or empty until scoped lookup is implemented + */ + default Optional getJob(UUID jobId, TenantContext tenantContext) { + return Optional.empty(); + } + /** * Retries a dead-lettered conversion job by moving it back to submitted state. * diff --git a/src/test/java/com/clearfolio/viewer/repository/ConversionJobRepositoryTest.java b/src/test/java/com/clearfolio/viewer/repository/ConversionJobRepositoryTest.java index 90b467ce0..3b3fb9022 100644 --- a/src/test/java/com/clearfolio/viewer/repository/ConversionJobRepositoryTest.java +++ b/src/test/java/com/clearfolio/viewer/repository/ConversionJobRepositoryTest.java @@ -62,8 +62,6 @@ public void deleteById(UUID jobId) { assertSame(job, repository.findByTenantAndContentHash("tenant-a", "hash").orElseThrow()); assertTrue(repository.findByTenantAndContentHash("tenant-b", "hash").isEmpty()); - assertSame(job, repository.findByTenantAndId("tenant-a", job.getJobId()).orElseThrow()); - assertTrue(repository.findByTenantAndId("tenant-b", job.getJobId()).isEmpty()); } @Test diff --git a/src/test/java/com/clearfolio/viewer/repository/TenantScopedJobLookupBoundaryTest.java b/src/test/java/com/clearfolio/viewer/repository/TenantScopedJobLookupBoundaryTest.java new file mode 100644 index 000000000..e970cf33a --- /dev/null +++ b/src/test/java/com/clearfolio/viewer/repository/TenantScopedJobLookupBoundaryTest.java @@ -0,0 +1,60 @@ +package com.clearfolio.viewer.repository; + +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.CALLS_REAL_METHODS; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; + +import java.util.Optional; +import java.util.UUID; + +import org.junit.jupiter.api.Test; + +import com.clearfolio.viewer.model.ConversionJob; + +/** + * Verifies that tenant-scoped identifier lookup never falls back to a global + * repository read before tenant ownership is applied. + */ +class TenantScopedJobLookupBoundaryTest { + + @Test + void repositoryDefaultTenantIdQueryFailsClosedWithoutGlobalLookup() { + ConversionJobRepository repository = mock(ConversionJobRepository.class, CALLS_REAL_METHODS); + UUID jobId = UUID.randomUUID(); + + assertTrue(repository.findByTenantAndId("tenant-a", jobId).isEmpty()); + + verify(repository, never()).findById(jobId); + } + + @Test + void inMemoryTenantIdQueryUsesScopedStorageBoundary() { + UUID jobId = UUID.randomUUID(); + ConversionJob job = new ConversionJob( + jobId, + "tenant-a", + "operator-a", + "tenant-a.docx", + "application/octet-stream", + "hash-a", + 42L, + 3 + ); + InMemoryConversionJobRepository repository = new InMemoryConversionJobRepository() { + @Override + public Optional findById(UUID ignoredJobId) { + throw new AssertionError("scoped lookup must not invoke global findById"); + } + }; + repository.save(job); + + assertTrue(repository.findByTenantAndId(null, jobId).isEmpty()); + assertTrue(repository.findByTenantAndId(" ", jobId).isEmpty()); + assertTrue(repository.findByTenantAndId("tenant-a", null).isEmpty()); + assertTrue(repository.findByTenantAndId("tenant-b", jobId).isEmpty()); + assertSame(job, repository.findByTenantAndId(" tenant-a ", jobId).orElseThrow()); + } +} diff --git a/src/test/java/com/clearfolio/viewer/service/TenantScopedConversionServiceLookupTest.java b/src/test/java/com/clearfolio/viewer/service/TenantScopedConversionServiceLookupTest.java new file mode 100644 index 000000000..c3c5f2347 --- /dev/null +++ b/src/test/java/com/clearfolio/viewer/service/TenantScopedConversionServiceLookupTest.java @@ -0,0 +1,70 @@ +package com.clearfolio.viewer.service; + +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.CALLS_REAL_METHODS; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.Optional; +import java.util.Set; +import java.util.UUID; + +import org.junit.jupiter.api.Test; + +import com.clearfolio.viewer.auth.TenantContext; +import com.clearfolio.viewer.config.ConversionProperties; +import com.clearfolio.viewer.model.ConversionJob; +import com.clearfolio.viewer.repository.ConversionJobRepository; + +/** + * Proves that application-service lookup can preserve tenant authority all the + * way to the repository lookup boundary instead of reading global job state + * first and filtering ownership afterward. + */ +class TenantScopedConversionServiceLookupTest { + + @Test + void interfaceDefaultFailsClosedWithoutGlobalLookup() { + DocumentConversionService service = mock(DocumentConversionService.class, CALLS_REAL_METHODS); + UUID jobId = UUID.randomUUID(); + TenantContext tenant = new TenantContext("tenant-a", "subject-a", Set.of()); + + assertTrue(service.getJob(jobId, tenant).isEmpty()); + + verify(service, never()).getJob(jobId); + } + + @Test + void defaultServiceDelegatesTenantPredicateToRepository() { + ConversionJobRepository repository = mock(ConversionJobRepository.class); + UUID jobId = UUID.randomUUID(); + ConversionJob job = new ConversionJob( + jobId, + "tenant-a", + "subject-a", + "report.pdf", + "application/pdf", + "hash-a", + 42L, + 3 + ); + TenantContext tenant = new TenantContext("tenant-a", "subject-a", Set.of()); + when(repository.findByTenantAndId("tenant-a", jobId)).thenReturn(Optional.of(job)); + DefaultDocumentConversionService service = new DefaultDocumentConversionService( + repository, + file -> { }, + ignoredJobId -> { }, + new ConversionProperties() + ); + + assertSame(job, service.getJob(jobId, tenant).orElseThrow()); + assertTrue(service.getJob(jobId, null).isEmpty()); + assertTrue(service.getJob(null, tenant).isEmpty()); + + verify(repository).findByTenantAndId("tenant-a", jobId); + verify(repository, never()).findById(jobId); + } +}