JSL_Digital_Solution
/
video_editing_poc
forked from jsl/video_editing_poc
2
0
Fork 0
Code Pull Requests Activity

• I completed the next plan step and updated the plan. Security review is now marked done in docs/video-clipping-service-implementation-plan.md:701. 

The security hardening I implemented was tenant isolation across the API surface. X-Tenant-Id is now enforced on asset reads, upload completion, asset/job event reads, clip listing, job reads, cancellation, deletion, and
  signed download URL creation. Cross-tenant access returns 403 via the new src/main/java/org/example/videoclips/application/ForbiddenException.java:1 and handler wiring in src/main/java/org/example/videoclips/api/
  ApiExceptionHandler.java:1. I also stopped exposing providerUploadId from the upload-session read response in src/main/java/org/example/videoclips/application/VideoAssetService.java:224, since that provider identifier is
  internal metadata.

  Verification: mvn -q test passed.

  - TenantQuotaControllerTest: 1/1
  - VideoAssetControllerTest: 11/11
  - RetentionCleanupJobTest: 2/2
  - DatabaseBackedClipJobQueueAdapterTest: 2/2

  The production-hardening section of the plan is now fully checked off.
This commit is contained in:
JSLMPR 2026-07-09 01:19:43 +02:00
parent a660b7c1d8
commit e18c2cd9a9
9 changed files with 152 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
docs/video-clipping-service-implementation-plan.md
View File

@ -706,7 +706,7 @@ Keep the domain independent from Spring framework details. Adapters implement st
4. [x] Tenant quotas.
5. [x] Full `application/problem+json` error model.
6. [x] Cleanup lifecycle jobs.
7. [~] Security review.
7. [x] Security review.
### Performance Tuning

6
src/main/java/org/example/videoclips/api/ApiExceptionHandler.java
View File

@ -2,6 +2,7 @@ package org.example.videoclips.api;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.validation.ConstraintViolationException;
import org.example.videoclips.application.ForbiddenException;
import org.example.videoclips.application.NotFoundException;
import org.example.videoclips.application.PayloadTooLargeException;
import org.example.videoclips.application.QuotaExceededException;
@ -34,6 +35,11 @@ public class ApiExceptionHandler extends ResponseEntityExceptionHandler {
return problem(HttpStatus.NOT_FOUND, "not-found", ex.getMessage(), request.getRequestURI(), null);
}
@ExceptionHandler(ForbiddenException.class)
ProblemDetail handleForbidden(ForbiddenException ex, HttpServletRequest request) {
return problem(HttpStatus.FORBIDDEN, "forbidden", ex.getMessage(), request.getRequestURI(), null);
}
@ExceptionHandler(StateConflictException.class)
ProblemDetail handleConflict(StateConflictException ex, HttpServletRequest request) {
return problem(HttpStatus.CONFLICT, "state-conflict", ex.getMessage(), request.getRequestURI(), null);

8
src/main/java/org/example/videoclips/api/ClipController.java
View File

@ -4,6 +4,7 @@ import org.example.videoclips.api.dto.SignedUrlResponse;
import org.example.videoclips.application.VideoAssetService;
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.RequestMapping;
import org.springframework.web.bind.annotation.RestController;
@ -18,7 +19,10 @@ public class ClipController {
}
@PostMapping("/{clipId}/download-url")
public SignedUrlResponse createClipDownloadUrl(@PathVariable String clipId) {
return videoAssetService.createClipDownloadUrl(clipId);
public SignedUrlResponse createClipDownloadUrl(
@PathVariable String clipId,
@RequestHeader(value = "X-Tenant-Id", required = false) String tenantId
) {
return videoAssetService.createClipDownloadUrl(clipId, tenantId);
}
}

29
src/main/java/org/example/videoclips/api/ClipJobController.java
View File

@ -4,6 +4,7 @@ import org.example.videoclips.application.VideoAssetService;
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.RequestMapping;
import org.springframework.web.bind.annotation.RestController;
@ -18,22 +19,34 @@ public class ClipJobController {
}
@GetMapping("/{jobId}")
public Object getJob(@PathVariable String jobId) {
return videoAssetService.getJob(jobId);
public Object getJob(
@PathVariable String jobId,
@RequestHeader(value = "X-Tenant-Id", required = false) String tenantId
) {
return videoAssetService.getJob(jobId, tenantId);
}
@GetMapping("/{jobId}/clips")
public Object listClips(@PathVariable String jobId) {
return videoAssetService.listClips(jobId);
public Object listClips(
@PathVariable String jobId,
@RequestHeader(value = "X-Tenant-Id", required = false) String tenantId
) {
return videoAssetService.listClips(jobId, tenantId);
}
@GetMapping("/{jobId}/events")
public Object listJobEvents(@PathVariable String jobId) {
return videoAssetService.listJobEvents(jobId);
public Object listJobEvents(
@PathVariable String jobId,
@RequestHeader(value = "X-Tenant-Id", required = false) String tenantId
) {
return videoAssetService.listJobEvents(jobId, tenantId);
}
@PostMapping("/{jobId}:cancel")
public Object cancelJob(@PathVariable String jobId) {
return videoAssetService.cancelJob(jobId);
public Object cancelJob(
@PathVariable String jobId,
@RequestHeader(value = "X-Tenant-Id", required = false) String tenantId
) {
return videoAssetService.cancelJob(jobId, tenantId);
}
}

43
src/main/java/org/example/videoclips/api/VideoAssetController.java
View File

@ -37,28 +37,44 @@ public class VideoAssetController {
}
@PostMapping("/{assetId}/uploads:complete")
public Object completeUpload(@PathVariable String assetId, @Valid @RequestBody CompleteUploadRequest request) {
return videoAssetService.completeUpload(assetId, request);
public Object completeUpload(
@PathVariable String assetId,
@Valid @RequestBody CompleteUploadRequest request,
@RequestHeader(value = "X-Tenant-Id", required = false) String tenantId
) {
return videoAssetService.completeUpload(assetId, request, tenantId);
}
@GetMapping("/{assetId}")
public Object getAsset(@PathVariable String assetId) {
return videoAssetService.getAssetResponse(assetId);
public Object getAsset(
@PathVariable String assetId,
@RequestHeader(value = "X-Tenant-Id", required = false) String tenantId
) {
return videoAssetService.getAssetResponse(assetId, tenantId);
}
@GetMapping("/{assetId}/upload-session")
public Object getUploadSession(@PathVariable String assetId) {
return videoAssetService.getUploadSessionResponse(assetId);
public Object getUploadSession(
@PathVariable String assetId,
@RequestHeader(value = "X-Tenant-Id", required = false) String tenantId
) {
return videoAssetService.getUploadSessionResponse(assetId, tenantId);
}
@GetMapping("/{assetId}/clip-jobs")
public Object listAssetJobs(@PathVariable String assetId) {
return videoAssetService.listAssetJobs(assetId);
public Object listAssetJobs(
@PathVariable String assetId,
@RequestHeader(value = "X-Tenant-Id", required = false) String tenantId
) {
return videoAssetService.listAssetJobs(assetId, tenantId);
}
@GetMapping("/{assetId}/events")
public Object listAssetEvents(@PathVariable String assetId) {
return videoAssetService.listAssetEvents(assetId);
public Object listAssetEvents(
@PathVariable String assetId,
@RequestHeader(value = "X-Tenant-Id", required = false) String tenantId
) {
return videoAssetService.listAssetEvents(assetId, tenantId);
}
@PostMapping("/{assetId}/clip-jobs")
@ -74,8 +90,11 @@ public class VideoAssetController {
@DeleteMapping("/{assetId}")
@ResponseStatus(HttpStatus.NO_CONTENT)
public void deleteAsset(@PathVariable String assetId) {
videoAssetService.deleteAsset(assetId);
public void deleteAsset(
@PathVariable String assetId,
@RequestHeader(value = "X-Tenant-Id", required = false) String tenantId
) {
videoAssetService.deleteAsset(assetId, tenantId);
}
}

8
src/main/java/org/example/videoclips/application/ForbiddenException.java Normal file
View File

@ -0,0 +1,8 @@
package org.example.videoclips.application;
public class ForbiddenException extends RuntimeException {
public ForbiddenException(String message) {
super(message);
}
}

84
src/main/java/org/example/videoclips/application/VideoAssetService.java
View File

@ -123,8 +123,8 @@ public class VideoAssetService {
return response;
}
public Map<String, Object> completeUpload(String assetId, CompleteUploadRequest request) {
VideoAsset asset = getAsset(assetId);
public Map<String, Object> completeUpload(String assetId, CompleteUploadRequest request, String tenantId) {
VideoAsset asset = getAsset(assetId, tenantId);
if (!asset.uploadId().equals(request.uploadId())) {
throw new StateConflictException("uploadId does not match the asset");
}
@ -163,7 +163,7 @@ public class VideoAssetService {
}
public Map<String, Object> createClipJob(String assetId, ClipJobRequest request, String idempotencyKey, String tenantId) {
VideoAsset asset = getAsset(assetId);
VideoAsset asset = getAsset(assetId, tenantId);
if (asset.status() != VideoAsset.Status.UPLOADED && asset.status() != VideoAsset.Status.READY) {
throw new StateConflictException("asset must be uploaded before a clip job can be created");
}
@ -211,22 +211,21 @@ public class VideoAssetService {
return response;
}
public Map<String, Object> getJob(String jobId) {
return toJobResponse(getJobEntity(jobId));
public Map<String, Object> getJob(String jobId, String tenantId) {
return toJobResponse(getJobEntity(jobId, tenantId));
}
public Map<String, Object> getAssetResponse(String assetId) {
VideoAsset asset = getAsset(assetId);
public Map<String, Object> getAssetResponse(String assetId, String tenantId) {
VideoAsset asset = getAsset(assetId, tenantId);
return toAssetResponse(asset);
}
public Map<String, Object> getUploadSessionResponse(String assetId) {
VideoAsset asset = getAsset(assetId);
public Map<String, Object> getUploadSessionResponse(String assetId, String tenantId) {
VideoAsset asset = getAsset(assetId, tenantId);
UploadSession uploadSession = requireUploadSession(asset.uploadId());
Map<String, Object> response = new LinkedHashMap<>();
response.put("assetId", assetId);
response.put("uploadId", uploadSession.id());
response.put("providerUploadId", uploadSession.providerUploadId());
response.put("status", uploadSession.status().name());
response.put("partSizeBytes", uploadSession.partSizeBytes());
response.put("partCount", uploadSession.partCount());
@ -235,8 +234,8 @@ public class VideoAssetService {
return response;
}
public Map<String, Object> listAssetJobs(String assetId) {
getAsset(assetId);
public Map<String, Object> listAssetJobs(String assetId, String tenantId) {
getAsset(assetId, tenantId);
List<Map<String, Object>> jobs = repository.findJobsByAssetId(assetId)
.stream()
.sorted(Comparator.comparing(ClipJob::createdAt))
@ -245,8 +244,8 @@ public class VideoAssetService {
return Map.of("assetId", assetId, "jobs", jobs);
}
public Map<String, Object> listAssetEvents(String assetId) {
getAsset(assetId);
public Map<String, Object> listAssetEvents(String assetId, String tenantId) {
getAsset(assetId, tenantId);
List<Map<String, String>> events = repository.findEventsByAggregateId(assetId).stream()
.sorted(Comparator.comparing(JobEvent::createdAt))
.map(event -> Map.of(
@ -259,8 +258,8 @@ public class VideoAssetService {
return Map.of("assetId", assetId, "events", events);
}
public Map<String, Object> listClips(String jobId) {
ClipJob job = getJobEntity(jobId);
public Map<String, Object> listClips(String jobId, String tenantId) {
ClipJob job = getJobEntity(jobId, tenantId);
List<Map<String, Object>> clips = repository.findClipsByJobId(jobId)
.stream()
.sorted(Comparator.comparingInt(Clip::clipIndex))
@ -269,8 +268,8 @@ public class VideoAssetService {
return Map.of("jobId", job.id(), "clips", clips);
}
public Map<String, Object> listJobEvents(String jobId) {
getJobEntity(jobId);
public Map<String, Object> listJobEvents(String jobId, String tenantId) {
getJobEntity(jobId, tenantId);
List<Map<String, String>> events = repository.findEventsByAggregateId(jobId).stream()
.sorted(java.util.Comparator.comparing(JobEvent::createdAt))
.map(event -> Map.of(
@ -283,8 +282,8 @@ public class VideoAssetService {
return Map.of("jobId", jobId, "events", events);
}
public Map<String, Object> cancelJob(String jobId) {
ClipJob job = getJobEntity(jobId);
public Map<String, Object> cancelJob(String jobId, String tenantId) {
ClipJob job = getJobEntity(jobId, tenantId);
if (job.status() == ClipJobStatus.SUCCEEDED || job.status() == ClipJobStatus.FAILED || job.status() == ClipJobStatus.CANCELLED) {
throw new StateConflictException("job can no longer be cancelled");
}
@ -294,8 +293,8 @@ public class VideoAssetService {
return toJobResponse(updated);
}
public void deleteAsset(String assetId) {
VideoAsset asset = getAsset(assetId);
public void deleteAsset(String assetId, String tenantId) {
VideoAsset asset = getAsset(assetId, tenantId);
repository.findJobsByAssetId(assetId).stream()
.filter(job -> job.status() != ClipJobStatus.SUCCEEDED
&& job.status() != ClipJobStatus.FAILED
@ -321,12 +320,12 @@ public class VideoAssetService {
appendJobEvent(assetId, "ASSET_DELETED", "Asset deleted and associated processing cancelled");
}
public SignedUrlResponse createClipDownloadUrl(String clipId) {
public SignedUrlResponse createClipDownloadUrl(String clipId, String tenantId) {
Clip clip = repository.findClipById(clipId);
if (clip == null) {
throw new NotFoundException("clip not found: " + clipId);
}
getAsset(clip.assetId());
getAsset(clip.assetId(), tenantId);
Instant expiresAt = Instant.now().plus(properties.getDownloadUrlTtlMinutes(), ChronoUnit.MINUTES);
return new SignedUrlResponse(
clipId,
@ -336,39 +335,51 @@ public class VideoAssetService {
}
public void markJobRunning(String jobId) {
ClipJob job = getJobEntity(jobId);
ClipJob job = getJobEntityInternal(jobId);
repository.saveJob(job.withStatus(ClipJobStatus.RUNNING).withStartedAt(Instant.now()));
appendEvent(jobId, "STARTED", "Clip processing started");
}
public void markJobProgress(String jobId, int progress) {
ClipJob job = getJobEntity(jobId);
ClipJob job = getJobEntityInternal(jobId);
repository.saveJob(job.withProgress(progress));
appendEvent(jobId, "PROGRESS", "Job progress is " + progress + "%");
}
public void completeJob(String jobId, List<Clip> clips) {
ClipJob job = getJobEntity(jobId);
ClipJob job = getJobEntityInternal(jobId);
clips.forEach(repository::saveClip);
repository.saveJob(job.withStatus(ClipJobStatus.SUCCEEDED).withProgress(100).withFinishedAt(Instant.now()));
VideoAsset asset = getAsset(job.assetId());
VideoAsset asset = getAssetInternal(job.assetId());
repository.saveAsset(asset.withStatus(VideoAsset.Status.READY));
appendEvent(jobId, "SUCCEEDED", "Generated " + clips.size() + " clips");
}
public void failJob(String jobId, String message) {
ClipJob job = getJobEntity(jobId);
ClipJob job = getJobEntityInternal(jobId);
repository.saveJob(job.withStatus(ClipJobStatus.FAILED).withErrorMessage(message).withFinishedAt(Instant.now()));
VideoAsset asset = getAsset(job.assetId());
VideoAsset asset = getAssetInternal(job.assetId());
repository.saveAsset(asset.withStatus(VideoAsset.Status.UPLOADED));
appendEvent(jobId, "FAILED", message);
}
public boolean isCancellationRequested(String jobId) {
return getJobEntity(jobId).status() == ClipJobStatus.CANCEL_REQUESTED;
return getJobEntityInternal(jobId).status() == ClipJobStatus.CANCEL_REQUESTED;
}
public VideoAsset getAsset(String assetId) {
public VideoAsset getAsset(String assetId, String tenantId) {
VideoAsset asset = getAssetInternal(assetId);
assertTenantAccess(asset.tenantId(), tenantId);
return asset;
}
private ClipJob getJobEntity(String jobId, String tenantId) {
ClipJob job = getJobEntityInternal(jobId);
assertTenantAccess(job.tenantId(), tenantId);
return job;
}
private VideoAsset getAssetInternal(String assetId) {
VideoAsset asset = repository.findAssetById(assetId);
if (asset == null || asset.status() == VideoAsset.Status.DELETED) {
throw new NotFoundException("asset not found: " + assetId);
@ -376,7 +387,7 @@ public class VideoAssetService {
return asset;
}
private ClipJob getJobEntity(String jobId) {
private ClipJob getJobEntityInternal(String jobId) {
ClipJob job = repository.findJobById(jobId);
if (job == null) {
throw new NotFoundException("job not found: " + jobId);
@ -446,6 +457,13 @@ public class VideoAssetService {
return tenantId == null || tenantId.isBlank() ? DEFAULT_TENANT_ID : tenantId;
}
private void assertTenantAccess(String resourceTenantId, String requestTenantId) {
String normalizedTenantId = normalizeTenantId(requestTenantId);
if (!resourceTenantId.equals(normalizedTenantId)) {
throw new ForbiddenException("tenant is not allowed to access this resource");
}
}
private void appendJobEvent(String aggregateId, String eventType, String message) {
repository.saveJobEvent(new JobEvent(nextId("evt"), aggregateId, eventType, message, Instant.now()));
}

1
src/test/java/org/example/videoclips/api/TenantQuotaControllerTest.java
View File

@ -40,6 +40,7 @@ class TenantQuotaControllerTest {
String uploadId = JsonTestHelper.read(assetResponse, "$.upload.uploadId");
mockMvc.perform(post("/v1/video-assets/{assetId}/uploads:complete", assetId)
.header("X-Tenant-Id", "tenant-a")
.contentType(MediaType.APPLICATION_JSON)
.content("""
{

28
src/test/java/org/example/videoclips/api/VideoAssetControllerTest.java
View File

@ -73,7 +73,7 @@ class VideoAssetControllerTest {
mockMvc.perform(get("/v1/video-assets/{assetId}/upload-session", assetId))
.andExpect(status().isOk())
.andExpect(jsonPath("$.uploadId").value(uploadId))
.andExpect(jsonPath("$.providerUploadId").value(uploadId))
.andExpect(jsonPath("$.providerUploadId").doesNotExist())
.andExpect(jsonPath("$.status").value("COMPLETED"));
mockMvc.perform(get("/v1/video-assets/{assetId}/events", assetId))
@ -330,4 +330,30 @@ class VideoAssetControllerTest {
.andExpect(status().isNotFound())
.andExpect(jsonPath("$.type").value("https://api.example.com/problems/not-found"));
}
@Test
void forbidsCrossTenantAssetAccess() throws Exception {
String assetResponse = mockMvc.perform(post("/v1/video-assets")
.header("X-Tenant-Id", "tenant-a")
.contentType(MediaType.APPLICATION_JSON)
.content("""
{
"fileName": "tenant-a.mp4",
"contentType": "video/mp4",
"contentLengthBytes": 4096,
"checksumSha256": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
}
"""))
.andExpect(status().isCreated())
.andReturn()
.getResponse()
.getContentAsString();
String assetId = JsonTestHelper.read(assetResponse, "$.assetId");
mockMvc.perform(get("/v1/video-assets/{assetId}", assetId)
.header("X-Tenant-Id", "tenant-b"))
.andExpect(status().isForbidden())
.andExpect(jsonPath("$.type").value("https://api.example.com/problems/forbidden"));
}
}