From 563948691e90f66d58e1713cf8c0d966c36cdbf4 Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+Frooodle@users.noreply.github.com.> Date: Thu, 19 Jun 2025 12:28:12 +0100 Subject: [PATCH] ensure random people cant cancel jobs --- .../common/service/JobExecutorService.java | 16 ++++ .../controller/AdminJobController.java | 82 +++++++++++++++++++ .../common/controller/JobController.java | 60 ++++---------- 3 files changed, 116 insertions(+), 42 deletions(-) create mode 100644 proprietary/src/main/java/stirling/software/proprietary/controller/AdminJobController.java diff --git a/common/src/main/java/stirling/software/common/service/JobExecutorService.java b/common/src/main/java/stirling/software/common/service/JobExecutorService.java index 8213a5c7e..64a9c86b1 100644 --- a/common/src/main/java/stirling/software/common/service/JobExecutorService.java +++ b/common/src/main/java/stirling/software/common/service/JobExecutorService.java @@ -102,6 +102,22 @@ public class JobExecutorService { // Store the job ID in the request for potential use by other components if (request != null) { request.setAttribute("jobId", jobId); + + // Also track this job ID in the user's session for authorization purposes + // This ensures users can only cancel their own jobs + if (request.getSession() != null) { + @SuppressWarnings("unchecked") + java.util.Set userJobIds = (java.util.Set) + request.getSession().getAttribute("userJobIds"); + + if (userJobIds == null) { + userJobIds = new java.util.concurrent.ConcurrentSkipListSet<>(); + request.getSession().setAttribute("userJobIds", userJobIds); + } + + userJobIds.add(jobId); + log.debug("Added job ID {} to user session", jobId); + } } // Determine which timeout to use diff --git a/proprietary/src/main/java/stirling/software/proprietary/controller/AdminJobController.java b/proprietary/src/main/java/stirling/software/proprietary/controller/AdminJobController.java new file mode 100644 index 000000000..4f5b500cd --- /dev/null +++ b/proprietary/src/main/java/stirling/software/proprietary/controller/AdminJobController.java @@ -0,0 +1,82 @@ +package stirling.software.proprietary.controller; + +import java.util.Map; + +import org.springframework.http.ResponseEntity; +import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.RestController; + +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; + +import stirling.software.common.model.job.JobStats; +import stirling.software.common.service.JobQueue; +import stirling.software.common.service.TaskManager; +import stirling.software.proprietary.audit.AuditEventType; +import stirling.software.proprietary.audit.AuditLevel; +import stirling.software.proprietary.audit.Audited; + +/** + * Admin controller for job management. These endpoints require admin privileges + * and provide insight into system jobs and queues. + */ +@RestController +@RequiredArgsConstructor +@Slf4j +public class AdminJobController { + + private final TaskManager taskManager; + private final JobQueue jobQueue; + + /** + * Get statistics about jobs in the system (admin only) + * + * @return Job statistics + */ + @GetMapping("/api/v1/admin/job/stats") + @PreAuthorize("hasRole('ROLE_ADMIN')") + public ResponseEntity getJobStats() { + JobStats stats = taskManager.getJobStats(); + log.info("Admin requested job stats: {} active, {} completed jobs", + stats.getActiveJobs(), stats.getCompletedJobs()); + return ResponseEntity.ok(stats); + } + + /** + * Get statistics about the job queue (admin only) + * + * @return Queue statistics + */ + @GetMapping("/api/v1/admin/job/queue/stats") + @PreAuthorize("hasRole('ROLE_ADMIN')") + public ResponseEntity getQueueStats() { + Map queueStats = jobQueue.getQueueStats(); + log.info("Admin requested queue stats: {} queued jobs", queueStats.get("queuedJobs")); + return ResponseEntity.ok(queueStats); + } + + /** + * Manually trigger cleanup of old jobs (admin only) + * + * @return A response indicating how many jobs were cleaned up + */ + @PostMapping("/api/v1/admin/job/cleanup") + @PreAuthorize("hasRole('ROLE_ADMIN')") + public ResponseEntity cleanupOldJobs() { + int beforeCount = taskManager.getJobStats().getTotalJobs(); + taskManager.cleanupOldJobs(); + int afterCount = taskManager.getJobStats().getTotalJobs(); + int removedCount = beforeCount - afterCount; + + log.info("Admin triggered job cleanup: removed {} jobs, {} remaining", + removedCount, afterCount); + + return ResponseEntity.ok( + Map.of( + "message", "Cleanup complete", + "removedJobs", removedCount, + "remainingJobs", afterCount)); + } +} \ No newline at end of file diff --git a/stirling-pdf/src/main/java/stirling/software/common/controller/JobController.java b/stirling-pdf/src/main/java/stirling/software/common/controller/JobController.java index 6f70f23e1..574a98616 100644 --- a/stirling-pdf/src/main/java/stirling/software/common/controller/JobController.java +++ b/stirling-pdf/src/main/java/stirling/software/common/controller/JobController.java @@ -6,9 +6,10 @@ import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.DeleteMapping; 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.RestController; +import jakarta.servlet.http.HttpServletRequest; + import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; @@ -27,6 +28,7 @@ public class JobController { private final TaskManager taskManager; private final FileStorage fileStorage; private final JobQueue jobQueue; + private final HttpServletRequest request; /** * Get the status of a job @@ -98,49 +100,13 @@ public class JobController { return ResponseEntity.ok(result.getResult()); } - /** - * Get statistics about jobs in the system - * - * @return Job statistics - */ - @GetMapping("/api/v1/general/job/stats") - public ResponseEntity getJobStats() { - JobStats stats = taskManager.getJobStats(); - return ResponseEntity.ok(stats); - } - - /** - * Get statistics about the job queue - * - * @return Queue statistics - */ - @GetMapping("/api/v1/general/job/queue/stats") - public ResponseEntity getQueueStats() { - Map queueStats = jobQueue.getQueueStats(); - return ResponseEntity.ok(queueStats); - } - - /** - * Manually trigger cleanup of old jobs - * - * @return A response indicating how many jobs were cleaned up - */ - @PostMapping("/api/v1/general/job/cleanup") - public ResponseEntity cleanupOldJobs() { - int beforeCount = taskManager.getJobStats().getTotalJobs(); - taskManager.cleanupOldJobs(); - int afterCount = taskManager.getJobStats().getTotalJobs(); - int removedCount = beforeCount - afterCount; - - return ResponseEntity.ok( - Map.of( - "message", "Cleanup complete", - "removedJobs", removedCount, - "remainingJobs", afterCount)); - } + // Admin-only endpoints have been moved to AdminJobController in the proprietary package /** * Cancel a job by its ID + * + * This method should only allow cancellation of jobs that were created by the current user. + * The jobId should be part of the user's session or otherwise linked to their identity. * * @param jobId The job ID * @return Response indicating whether the job was cancelled @@ -148,7 +114,17 @@ public class JobController { @DeleteMapping("/api/v1/general/job/{jobId}") public ResponseEntity cancelJob(@PathVariable("jobId") String jobId) { log.debug("Request to cancel job: {}", jobId); - + + // Verify that this job belongs to the current user + // We can use the current request's session to validate ownership + Object sessionJobIds = request.getSession().getAttribute("userJobIds"); + if (sessionJobIds == null || !(sessionJobIds instanceof java.util.Set) || + !((java.util.Set) sessionJobIds).contains(jobId)) { + // Either no jobs in session or jobId doesn't match user's jobs + log.warn("Unauthorized attempt to cancel job: {}", jobId); + return ResponseEntity.status(403).body(Map.of("message", "You are not authorized to cancel this job")); + } + // First check if the job is in the queue boolean cancelled = false; int queuePosition = -1;