diff --git a/src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java b/src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java index 79a972e3d..7d392036f 100644 --- a/src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java +++ b/src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java @@ -183,15 +183,20 @@ public ArtifactLinkResponse createLink( } /** - * Verifies a token before serving artifact bytes. + * Authenticates a read token before any document or artifact lookup occurs. + * + *

This phase verifies token syntax, signature, lifetime, read scope, + * document binding, issued-link authority, and revocation state without + * requiring conversion-job or artifact bytes. Callers must still use + * {@link #verifyReadToken(UUID, ConversionJob, byte[], String)} after lookup + * to verify tenant and artifact-checksum binding before returning bytes. * * @param docId route document identifier - * @param job conversion job - * @param artifactBytes stored artifact bytes * @param token supplied artifact token - * @return verified artifact token claims + * @return preauthorized artifact token claims + * @throws ArtifactTokenException when token authority is absent or invalid */ - public ArtifactTokenClaims verifyReadToken(UUID docId, ConversionJob job, byte[] artifactBytes, String token) { + public ArtifactTokenClaims preauthorizeReadToken(UUID docId, String token) { if (token == null || token.isBlank()) { throw new ArtifactTokenException(HttpStatus.UNAUTHORIZED, "artifact token required"); } @@ -216,6 +221,20 @@ public ArtifactTokenClaims verifyReadToken(UUID docId, ConversionJob job, byte[] || !record.artifactChecksum().equals(claims.artifactChecksum())) { throw new ArtifactTokenException(HttpStatus.FORBIDDEN, "artifact token ledger mismatch"); } + return claims; + } + + /** + * Verifies a token before serving artifact bytes. + * + * @param docId route document identifier + * @param job conversion job + * @param artifactBytes stored artifact bytes + * @param token supplied artifact token + * @return verified artifact token claims + */ + public ArtifactTokenClaims verifyReadToken(UUID docId, ConversionJob job, byte[] artifactBytes, String token) { + ArtifactTokenClaims claims = preauthorizeReadToken(docId, token); if (job == null || !job.belongsToTenant(claims.tenantId())) { throw new ArtifactTokenException(HttpStatus.FORBIDDEN, "artifact token tenant mismatch"); } diff --git a/src/main/java/com/clearfolio/viewer/controller/ArtifactController.java b/src/main/java/com/clearfolio/viewer/controller/ArtifactController.java index 881b98f37..ca754ee00 100644 --- a/src/main/java/com/clearfolio/viewer/controller/ArtifactController.java +++ b/src/main/java/com/clearfolio/viewer/controller/ArtifactController.java @@ -136,6 +136,13 @@ public Mono> getPdf( @RequestParam(value = ArtifactLinkService.ARTIFACT_TOKEN_PARAM, required = false) String queryToken, @RequestHeader(value = HttpHeaders.AUTHORIZATION, required = false) String authorizationHeader, @RequestHeader(value = "X-Request-Id", required = false) String traceId) { + String token = ArtifactLinkService.resolveToken(queryToken, authorizationHeader); + try { + artifactLinkService.preauthorizeReadToken(docId, token); + } catch (ArtifactTokenException ex) { + return Mono.just(ArtifactHttpResponse.tokenFailure(ex.getStatus())); + } + Optional job = conversionService.getJob(docId); if (job.isEmpty() || job.get().getStatus() != ConversionJobStatus.SUCCEEDED) { return Mono.just(ResponseEntity.status(HttpStatus.NOT_FOUND).build()); @@ -147,7 +154,6 @@ public Mono> getPdf( } byte[] pdfBytes = stored.get(); - String token = ArtifactLinkService.resolveToken(queryToken, authorizationHeader); ArtifactTokenClaims claims; try { claims = artifactLinkService.verifyReadToken(docId, job.get(), pdfBytes, token); diff --git a/src/test/java/com/clearfolio/viewer/controller/ArtifactControllerPostLookupBindingTest.java b/src/test/java/com/clearfolio/viewer/controller/ArtifactControllerPostLookupBindingTest.java new file mode 100644 index 000000000..ddff6c7e4 --- /dev/null +++ b/src/test/java/com/clearfolio/viewer/controller/ArtifactControllerPostLookupBindingTest.java @@ -0,0 +1,84 @@ +package com.clearfolio.viewer.controller; + +import static org.mockito.Mockito.mock; +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 org.springframework.test.web.reactive.server.WebTestClient; + +import com.clearfolio.viewer.api.ArtifactLinkRequest; +import com.clearfolio.viewer.api.ArtifactLinkResponse; +import com.clearfolio.viewer.artifact.ArtifactLinkService; +import com.clearfolio.viewer.artifact.ArtifactStore; +import com.clearfolio.viewer.artifact.InMemoryArtifactStore; +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; + +/** + * Verifies that token preauthorization does not replace the post-lookup tenant + * and artifact binding check performed immediately before bytes are returned. + */ +class ArtifactControllerPostLookupBindingTest { + + @Test + void rejectsPreauthorizedTokenWhenResolvedJobBelongsToAnotherTenant() { + UUID docId = UUID.randomUUID(); + byte[] pdfBytes = new byte[] {0, 1, 2, 3}; + ArtifactStore artifactStore = new InMemoryArtifactStore(); + artifactStore.putPdf(docId, pdfBytes); + ArtifactLinkService artifactLinkService = new ArtifactLinkService(artifactStore, "test-secret"); + + ConversionJob authorizedJob = succeededJob(docId, TenantContext.DEMO_TENANT_ID); + ArtifactLinkResponse link = artifactLinkService.createLink( + authorizedJob, + tenantContext(), + ArtifactLinkRequest.viewerPreview() + ); + + DocumentConversionService conversionService = mock(DocumentConversionService.class); + when(conversionService.getJob(docId)).thenReturn(Optional.of(succeededJob(docId, "other-tenant"))); + ArtifactController controller = new ArtifactController( + conversionService, + artifactStore, + artifactLinkService, + new TenantAccessService() + ); + + WebTestClient.bindToController(controller) + .build() + .get() + .uri(link.artifactUrl()) + .exchange() + .expectStatus().isForbidden(); + } + + private static TenantContext tenantContext() { + return new TenantContext( + TenantContext.DEMO_TENANT_ID, + TenantContext.DEMO_SUBJECT_ID, + Set.of(TenantPermissions.ARTIFACT_LINK_CREATE, TenantPermissions.VIEWER_READ) + ); + } + + private static ConversionJob succeededJob(UUID docId, String tenantId) { + ConversionJob job = new ConversionJob( + docId, + tenantId, + TenantContext.DEMO_SUBJECT_ID, + "report.docx", + "application/octet-stream", + "hash", + 4L, + 1 + ); + job.markSucceeded("/artifacts/" + docId + ".pdf", "done"); + return job; + } +} diff --git a/src/test/java/com/clearfolio/viewer/controller/ArtifactControllerTest.java b/src/test/java/com/clearfolio/viewer/controller/ArtifactControllerTest.java index 60231c7fe..d4a3cbc07 100644 --- a/src/test/java/com/clearfolio/viewer/controller/ArtifactControllerTest.java +++ b/src/test/java/com/clearfolio/viewer/controller/ArtifactControllerTest.java @@ -50,10 +50,13 @@ void setUp() { @Test void returnsNotFoundWhenJobMissing() { UUID docId = UUID.randomUUID(); + ConversionJob authorizedJob = succeededJob(docId); + artifactStore.putPdf(docId, sampleBytes()); + String artifactUrl = signedArtifactUrl(authorizedJob); when(conversionService.getJob(docId)).thenReturn(Optional.empty()); webTestClient.get() - .uri("/artifacts/{docId}.pdf", docId) + .uri(artifactUrl) .exchange() .expectStatus().isNotFound(); } @@ -61,6 +64,9 @@ void returnsNotFoundWhenJobMissing() { @Test void returnsNotFoundWhenJobIsNotSucceeded() { UUID docId = UUID.randomUUID(); + ConversionJob authorizedJob = succeededJob(docId); + artifactStore.putPdf(docId, sampleBytes()); + String artifactUrl = signedArtifactUrl(authorizedJob); ConversionJob job = new ConversionJob( docId, "report.docx", @@ -72,7 +78,7 @@ void returnsNotFoundWhenJobIsNotSucceeded() { when(conversionService.getJob(docId)).thenReturn(Optional.of(job)); webTestClient.get() - .uri("/artifacts/{docId}.pdf", docId) + .uri(artifactUrl) .exchange() .expectStatus().isNotFound(); } @@ -81,10 +87,13 @@ void returnsNotFoundWhenJobIsNotSucceeded() { void returnsNotFoundWhenArtifactIsMissing() { UUID docId = UUID.randomUUID(); ConversionJob job = succeededJob(docId); + artifactStore.putPdf(docId, sampleBytes()); + String artifactUrl = signedArtifactUrl(job); + artifactStore.deletePdf(docId); when(conversionService.getJob(docId)).thenReturn(Optional.of(job)); webTestClient.get() - .uri("/artifacts/{docId}.pdf", docId) + .uri(artifactUrl) .exchange() .expectStatus().isNotFound(); } diff --git a/src/test/java/com/clearfolio/viewer/controller/ArtifactControllerTokenPreconditionTest.java b/src/test/java/com/clearfolio/viewer/controller/ArtifactControllerTokenPreconditionTest.java new file mode 100644 index 000000000..1fe1fa870 --- /dev/null +++ b/src/test/java/com/clearfolio/viewer/controller/ArtifactControllerTokenPreconditionTest.java @@ -0,0 +1,91 @@ +package com.clearfolio.viewer.controller; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verifyNoInteractions; + +import java.util.UUID; + +import org.junit.jupiter.api.Test; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; + +import com.clearfolio.viewer.artifact.ArtifactLinkService; +import com.clearfolio.viewer.artifact.ArtifactStore; +import com.clearfolio.viewer.auth.TenantAccessService; +import com.clearfolio.viewer.service.DocumentConversionService; + +/** + * Proves that the public artifact route authenticates supplied credentials before + * reading conversion-job or artifact state that could become an existence oracle. + */ +class ArtifactControllerTokenPreconditionTest { + + @Test + void missingArtifactTokenIsRejectedBeforeSensitiveObjectLookup() { + Fixture fixture = fixture(); + + ResponseEntity response = fixture.controller().getPdf( + UUID.randomUUID(), null, null, null, null).block(); + + assertUnauthorizedWithoutObjectLookup(response, fixture); + } + + @Test + void unsupportedAuthorizationSchemeIsRejectedBeforeSensitiveObjectLookup() { + Fixture fixture = fixture(); + + ResponseEntity response = fixture.controller().getPdf( + UUID.randomUUID(), null, " ", "Basic opaque-credential", null).block(); + + assertUnauthorizedWithoutObjectLookup(response, fixture); + } + + @Test + void invalidQueryTokenIsRejectedBeforeSensitiveObjectLookup() { + Fixture fixture = fixture(); + + ResponseEntity response = fixture.controller().getPdf( + UUID.randomUUID(), null, "opaque-present-token", null, null).block(); + + assertUnauthorizedWithoutObjectLookup(response, fixture); + } + + @Test + void invalidBearerTokenIsRejectedBeforeSensitiveObjectLookup() { + Fixture fixture = fixture(); + + ResponseEntity response = fixture.controller().getPdf( + UUID.randomUUID(), null, null, "Bearer opaque-present-token", null).block(); + + assertUnauthorizedWithoutObjectLookup(response, fixture); + } + + private static Fixture fixture() { + DocumentConversionService conversionService = mock(DocumentConversionService.class); + ArtifactStore artifactStore = mock(ArtifactStore.class); + ArtifactLinkService artifactLinkService = new ArtifactLinkService(artifactStore, "test-secret"); + ArtifactController controller = new ArtifactController( + conversionService, + artifactStore, + artifactLinkService, + mock(TenantAccessService.class) + ); + return new Fixture(controller, conversionService, artifactStore); + } + + private static void assertUnauthorizedWithoutObjectLookup( + ResponseEntity response, + Fixture fixture) { + assertNotNull(response); + assertEquals(HttpStatus.UNAUTHORIZED, response.getStatusCode()); + verifyNoInteractions(fixture.conversionService(), fixture.artifactStore()); + } + + private record Fixture( + ArtifactController controller, + DocumentConversionService conversionService, + ArtifactStore artifactStore) { + } +}