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 4d50673ea..a639771a1 100644 --- a/src/main/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepository.java +++ b/src/main/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepository.java @@ -40,16 +40,12 @@ public class InMemoryConversionJobRepository implements ConversionJobRepository, private final ConcurrentLinkedQueue lifecycleEvents = new ConcurrentLinkedQueue<>(); private final Object jobIndexLock = new Object(); - /** - * Creates an empty thread-safe process-local job repository and lifecycle ledger. - */ + /** Creates an empty thread-safe process-local job repository and lifecycle ledger. */ public InMemoryConversionJobRepository() { // Concurrent collections are initialized eagerly for immediate multi-worker use. } - /** - * {@inheritDoc} - */ + /** {@inheritDoc} */ @Override public ConversionJob save(ConversionJob job) { Objects.requireNonNull(job, "job"); @@ -74,9 +70,7 @@ public ConversionJob save(ConversionJob job) { } } - /** - * {@inheritDoc} - */ + /** {@inheritDoc} */ @Override public ConversionJobRepository.FindOrStoreResult findOrStoreByContentHash(ConversionJob candidate) { Objects.requireNonNull(candidate, "candidate"); @@ -111,25 +105,19 @@ public ConversionJobRepository.FindOrStoreResult findOrStoreByContentHash(Conver } } - /** - * {@inheritDoc} - */ + /** {@inheritDoc} */ @Override public Optional findById(UUID jobId) { return Optional.ofNullable(jobs.get(jobId)); } - /** - * {@inheritDoc} - */ + /** {@inheritDoc} */ @Override public Optional findByContentHash(String contentHash) { return findByTenantAndContentHash("buyer-demo", contentHash); } - /** - * {@inheritDoc} - */ + /** {@inheritDoc} */ @Override public Optional findByTenantAndContentHash(String tenantId, String contentHash) { if (contentHash == null || contentHash.isBlank()) { @@ -144,17 +132,24 @@ public Optional findByTenantAndContentHash(String tenantId, Strin return findById(jobId); } - /** - * {@inheritDoc} - */ + /** {@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} */ @Override public List findAll() { return List.copyOf(jobs.values()); } - /** - * {@inheritDoc} - */ + /** {@inheritDoc} */ @Override public List findAllByTenantId(String tenantId) { if (tenantId == null || tenantId.isBlank()) { @@ -166,9 +161,7 @@ public List findAllByTenantId(String tenantId) { .toList(); } - /** - * {@inheritDoc} - */ + /** {@inheritDoc} */ @Override public void deleteById(UUID jobId) { synchronized (jobIndexLock) { @@ -207,9 +200,7 @@ public List findLifecycleEventsByTenantId(String te .toList(); } - /** - * {@inheritDoc} - */ + /** {@inheritDoc} */ @Override public Optional claimForProcessing(UUID jobId, Instant now) { Optional job = findById(jobId); @@ -226,9 +217,7 @@ public Optional claimForProcessing(UUID jobId, Instant now) { return job; } - /** - * {@inheritDoc} - */ + /** {@inheritDoc} */ @Override public void scheduleRetry(UUID jobId, String message, Instant retryAt) { findById(jobId).ifPresent(job -> { @@ -238,9 +227,7 @@ public void scheduleRetry(UUID jobId, String message, Instant retryAt) { }); } - /** - * {@inheritDoc} - */ + /** {@inheritDoc} */ @Override public void markSucceeded(UUID jobId, String resourcePath, String message) { findById(jobId).ifPresent(job -> { @@ -250,9 +237,7 @@ public void markSucceeded(UUID jobId, String resourcePath, String message) { }); } - /** - * {@inheritDoc} - */ + /** {@inheritDoc} */ @Override public void markDeadLettered(UUID jobId, String message) { findById(jobId).ifPresent(job -> { @@ -265,9 +250,7 @@ public void markDeadLettered(UUID jobId, String message) { }); } - /** - * {@inheritDoc} - */ + /** {@inheritDoc} */ @Override public boolean retryDeadLettered(UUID jobId, String operatorId) { Optional job = findById(jobId); 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/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); + } +}