mirror of
https://github.com/Frooodle/Stirling-PDF.git
synced 2026-04-22 23:08:53 +02:00
hardening (#5807)
Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -262,17 +262,30 @@ public class JobController {
|
||||
@Operation(summary = "Get file metadata")
|
||||
public ResponseEntity<?> getFileMetadata(@PathVariable("fileId") String fileId) {
|
||||
try {
|
||||
// Verify file exists
|
||||
if (!fileStorage.fileExists(fileId)) {
|
||||
String jobKey = taskManager.findJobKeyByFileId(fileId);
|
||||
if (jobKey == null) {
|
||||
return ResponseEntity.notFound().build();
|
||||
}
|
||||
|
||||
if (!validateJobAccess(jobKey)) {
|
||||
log.warn("Unauthorized attempt to access file metadata: {}", fileId);
|
||||
return ResponseEntity.status(403)
|
||||
.body(Map.of("message", "You are not authorized to access this file"));
|
||||
}
|
||||
|
||||
// Find the file metadata from any job that contains this file
|
||||
ResultFile resultFile = taskManager.findResultFileByFileId(fileId);
|
||||
|
||||
if (resultFile != null) {
|
||||
return ResponseEntity.ok(resultFile);
|
||||
} else {
|
||||
}
|
||||
|
||||
if (!isSecurityEnabled()) {
|
||||
// Backwards compatibility when ownership service is unavailable
|
||||
if (!fileStorage.fileExists(fileId)) {
|
||||
return ResponseEntity.notFound().build();
|
||||
}
|
||||
|
||||
// File exists but no metadata found, get basic info efficiently
|
||||
long fileSize = fileStorage.getFileSize(fileId);
|
||||
return ResponseEntity.ok(
|
||||
@@ -286,6 +299,8 @@ public class JobController {
|
||||
"fileSize",
|
||||
fileSize));
|
||||
}
|
||||
|
||||
return ResponseEntity.notFound().build();
|
||||
} catch (Exception e) {
|
||||
log.error("Error retrieving file metadata {}: {}", fileId, e.getMessage(), e);
|
||||
return ResponseEntity.internalServerError()
|
||||
@@ -303,11 +318,17 @@ public class JobController {
|
||||
@Operation(summary = "Download a file")
|
||||
public ResponseEntity<?> downloadFile(@PathVariable("fileId") String fileId) {
|
||||
try {
|
||||
// Verify file exists
|
||||
if (!fileStorage.fileExists(fileId)) {
|
||||
String jobKey = taskManager.findJobKeyByFileId(fileId);
|
||||
if (jobKey == null) {
|
||||
return ResponseEntity.notFound().build();
|
||||
}
|
||||
|
||||
if (!validateJobAccess(jobKey)) {
|
||||
log.warn("Unauthorized attempt to download file: {}", fileId);
|
||||
return ResponseEntity.status(403)
|
||||
.body(Map.of("message", "You are not authorized to access this file"));
|
||||
}
|
||||
|
||||
// Retrieve file content
|
||||
byte[] fileContent = fileStorage.retrieveBytes(fileId);
|
||||
|
||||
@@ -327,11 +348,14 @@ public class JobController {
|
||||
.body(fileContent);
|
||||
} catch (Exception e) {
|
||||
log.error("Error retrieving file {}: {}", fileId, e.getMessage(), e);
|
||||
return ResponseEntity.internalServerError()
|
||||
.body("Error retrieving file: " + e.getMessage());
|
||||
return ResponseEntity.internalServerError().body("Error retrieving file");
|
||||
}
|
||||
}
|
||||
|
||||
private boolean isSecurityEnabled() {
|
||||
return jobOwnershipService != null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Create Content-Disposition header with UTF-8 filename support
|
||||
*
|
||||
|
||||
@@ -15,11 +15,13 @@ import org.springframework.http.HttpStatus;
|
||||
import org.springframework.http.MediaType;
|
||||
import org.springframework.http.ResponseEntity;
|
||||
import org.springframework.mock.web.MockHttpSession;
|
||||
import org.springframework.test.util.ReflectionTestUtils;
|
||||
|
||||
import jakarta.servlet.http.HttpServletRequest;
|
||||
|
||||
import stirling.software.common.model.job.JobResult;
|
||||
import stirling.software.common.service.FileStorage;
|
||||
import stirling.software.common.service.JobOwnershipService;
|
||||
import stirling.software.common.service.JobQueue;
|
||||
import stirling.software.common.service.TaskManager;
|
||||
|
||||
@@ -33,6 +35,8 @@ class JobControllerTest {
|
||||
|
||||
@Mock private HttpServletRequest request;
|
||||
|
||||
@Mock private JobOwnershipService jobOwnershipService;
|
||||
|
||||
private MockHttpSession session;
|
||||
|
||||
@InjectMocks private JobController controller;
|
||||
@@ -404,4 +408,32 @@ class JobControllerTest {
|
||||
|
||||
verify(taskManager).setError(jobId, "Job was cancelled by user");
|
||||
}
|
||||
|
||||
@Test
|
||||
void testDownloadFile_ForbiddenWhenFileOwnedByAnotherUser() throws Exception {
|
||||
String fileId = "file-id";
|
||||
|
||||
ReflectionTestUtils.setField(controller, "jobOwnershipService", jobOwnershipService);
|
||||
when(taskManager.findJobKeyByFileId(fileId)).thenReturn("other-user:job-id");
|
||||
when(jobOwnershipService.validateJobAccess("other-user:job-id")).thenReturn(false);
|
||||
|
||||
ResponseEntity<?> response = controller.downloadFile(fileId);
|
||||
|
||||
assertEquals(HttpStatus.FORBIDDEN, response.getStatusCode());
|
||||
verify(fileStorage, never()).retrieveBytes(eq(fileId));
|
||||
}
|
||||
|
||||
@Test
|
||||
void testGetFileMetadata_ForbiddenWhenFileOwnedByAnotherUser() throws Exception {
|
||||
String fileId = "file-id";
|
||||
|
||||
ReflectionTestUtils.setField(controller, "jobOwnershipService", jobOwnershipService);
|
||||
when(taskManager.findJobKeyByFileId(fileId)).thenReturn("other-user:job-id");
|
||||
when(jobOwnershipService.validateJobAccess("other-user:job-id")).thenReturn(false);
|
||||
|
||||
ResponseEntity<?> response = controller.getFileMetadata(fileId);
|
||||
|
||||
assertEquals(HttpStatus.FORBIDDEN, response.getStatusCode());
|
||||
verify(fileStorage, never()).getFileSize(eq(fileId));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user