From 7b433dec2276fa41d541cc8480e867f231641d26 Mon Sep 17 00:00:00 2001 From: Anthony Stirling Date: Sat, 13 Jun 2026 21:08:16 +0100 Subject: [PATCH] Fix remaining cucumber regression failures in storage, user and form endpoints --- .../api/form/FormFillController.java | 31 +++-- .../exception/GlobalExceptionHandler.java | 67 +++++++++++ .../controller/api/UserController.java | 112 ++++++++++++++---- .../security/service/UserService.java | 13 +- .../FileFolderPlacementController.java | 91 -------------- .../controller/FileStorageController.java | 76 ++++++++++++ .../storage/service/FileStorageService.java | 25 ++-- .../storage/service/FolderService.java | 6 +- 8 files changed, 285 insertions(+), 136 deletions(-) delete mode 100644 app/proprietary/src/main/java/stirling/software/proprietary/storage/controller/FileFolderPlacementController.java diff --git a/app/core/src/main/java/stirling/software/SPDF/controller/api/form/FormFillController.java b/app/core/src/main/java/stirling/software/SPDF/controller/api/form/FormFillController.java index b97d2c04ea..20b395230e 100644 --- a/app/core/src/main/java/stirling/software/SPDF/controller/api/form/FormFillController.java +++ b/app/core/src/main/java/stirling/software/SPDF/controller/api/form/FormFillController.java @@ -86,11 +86,24 @@ public class FormFillController { } } - private static String decodePart(byte[] payload) { - if (payload == null || payload.length == 0) { + // Read a JSON/text multipart part as a raw UTF-8 string. The part is bound as FileUpload rather + // than byte[]/String on purpose: clients send these parts with Content-Type: application/json, + // and RESTEasy then routes a byte[]/String target through the Jackson reader, which fails + // trying + // to deserialize a JSON object (e.g. "{}") into those types and surfaces as a 500. FileUpload + // is + // always read verbatim, so the raw bytes reach the parser below regardless of the part's + // declared content type. An absent or empty part yields null (the no-op path). + private static String decodePart(FileUpload upload) throws IOException { + MultipartFile part = FileUploadMultipartFile.of(upload); + if (part == null || part.isEmpty()) { return null; } - return new String(payload, StandardCharsets.UTF_8); + byte[] bytes = part.getBytes(); + if (bytes == null || bytes.length == 0) { + return null; + } + return new String(bytes, StandardCharsets.UTF_8); } @POST @@ -241,11 +254,11 @@ public class FormFillController { description = "Updates existing fields in the provided PDF and returns the updated file") public Response modifyFields( - @RestForm("file") FileUpload fileUpload, @RestForm("updates") byte[] updatesPayload) + @RestForm("file") FileUpload fileUpload, @RestForm("updates") FileUpload updatesUpload) throws IOException { MultipartFile file = FileUploadMultipartFile.of(fileUpload); - String rawUpdates = decodePart(updatesPayload); + String rawUpdates = decodePart(updatesUpload); List modifications = FormPayloadParser.parseModificationDefinitions(objectMapper, rawUpdates); if (modifications.isEmpty()) { @@ -266,11 +279,11 @@ public class FormFillController { summary = "Delete form fields", description = "Removes the specified fields from the PDF and returns the updated file") public Response deleteFields( - @RestForm("file") FileUpload fileUpload, @RestForm("names") byte[] namesPayload) + @RestForm("file") FileUpload fileUpload, @RestForm("names") FileUpload namesUpload) throws IOException { MultipartFile file = FileUploadMultipartFile.of(fileUpload); - String rawNames = decodePart(namesPayload); + String rawNames = decodePart(namesUpload); List names = FormPayloadParser.parseNameList(objectMapper, rawNames); if (names.isEmpty()) { throw ExceptionUtils.createIllegalArgumentException( @@ -291,12 +304,12 @@ public class FormFillController { + " and returns the filled PDF") public Response fillForm( @RestForm("file") FileUpload fileUpload, - @RestForm("data") byte[] valuesPayload, + @RestForm("data") FileUpload dataUpload, @RestForm("flatten") @DefaultValue("false") boolean flatten) throws IOException { MultipartFile file = FileUploadMultipartFile.of(fileUpload); - String rawValues = decodePart(valuesPayload); + String rawValues = decodePart(dataUpload); Map values = FormPayloadParser.parseValueMap(objectMapper, rawValues); return processSingleFile( diff --git a/app/core/src/main/java/stirling/software/SPDF/exception/GlobalExceptionHandler.java b/app/core/src/main/java/stirling/software/SPDF/exception/GlobalExceptionHandler.java index afb9e7b074..5afc1c73b9 100644 --- a/app/core/src/main/java/stirling/software/SPDF/exception/GlobalExceptionHandler.java +++ b/app/core/src/main/java/stirling/software/SPDF/exception/GlobalExceptionHandler.java @@ -8,6 +8,7 @@ import java.util.Map; import java.util.ResourceBundle; import jakarta.enterprise.context.ApplicationScoped; +import jakarta.ws.rs.WebApplicationException; import jakarta.ws.rs.core.Context; import jakarta.ws.rs.core.Response; import jakarta.ws.rs.core.UriInfo; @@ -139,6 +140,17 @@ public class GlobalExceptionHandler implements ExceptionMapper { public Response toResponse(Throwable exception) { String requestUri = requestUri(); + // A WebApplicationException carries an explicit HTTP status the caller chose (the Quarkus + // equivalent of Spring's ResponseStatusException - see the class-level TODO). It must be + // honoured rather than collapsed into a generic 500 by the RuntimeException catch-all + // below: + // application code throws e.g. new WebApplicationException("...", BAD_REQUEST) to signal a + // 400/401/403/409, and that intent has to survive. Framework routing failures (404/405) are + // resolved before invocation and never reach this mapper, so this does not affect them. + if (exception instanceof WebApplicationException ex) { + return handleWebApplicationException(ex, requestUri); + } + if (exception instanceof PdfPasswordException ex) { return handlePdfPassword(ex, requestUri); } @@ -481,6 +493,56 @@ public class GlobalExceptionHandler implements ExceptionMapper { requestUri); } + /** + * Handle a JAX-RS {@link WebApplicationException}, preserving the HTTP status code the thrower + * embedded in it and wrapping the message in the standard RFC 7807 problem body. Replaces + * Spring's {@code ResponseStatusException} handling: callers across the app (e.g. {@code + * FolderService}, {@code FileStorageService}) throw {@code new WebApplicationException(message, + * status)} to signal a deliberate 4xx, and the original status must be returned verbatim. + * + * @param ex the WebApplicationException carrying the intended status + * @param requestUri the resolved request path + * @return a Response with the embedded status and a problem+json body + */ + public Response handleWebApplicationException(WebApplicationException ex, String requestUri) { + Response embedded = ex.getResponse(); + int statusCode = + embedded != null + ? embedded.getStatus() + : Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(); + Response.Status status = Response.Status.fromStatusCode(statusCode); + String reasonPhrase = status != null ? status.getReasonPhrase() : "HTTP " + statusCode; + + if (statusCode >= 500) { + log.error("WebApplicationException at {}: {}", requestUri, ex.getMessage(), ex); + } else { + log.warn( + "WebApplicationException at {}: {} ({})", + requestUri, + ex.getMessage(), + statusCode); + } + + String detail = ex.getMessage(); + if (detail == null || detail.isBlank()) { + detail = reasonPhrase; + } + + Map problemDetail = new LinkedHashMap<>(); + problemDetail.put("status", statusCode); + problemDetail.put("detail", detail); + problemDetail.put("timestamp", java.time.Instant.now()); + problemDetail.put("path", requestUri); + problemDetail.put( + "type", + status != null + ? "/errors/" + status.name().toLowerCase(Locale.ROOT).replace('_', '-') + : "/errors/http-" + statusCode); + problemDetail.put("title", reasonPhrase); + + return Response.status(statusCode).type(PROBLEM_JSON).entity(problemDetail).build(); + } + // =========================================================================================== // 406 NOT ACCEPTABLE - direct write // =========================================================================================== @@ -569,6 +631,11 @@ public class GlobalExceptionHandler implements ExceptionMapper { // Check if this RuntimeException wraps a typed exception from job execution Throwable cause = ex.getCause(); + if (cause instanceof WebApplicationException waEx) { + // A deliberate status thrown deeper in the call stack and rewrapped (e.g. by + // AutoJobAspect) must still surface with its intended code, not as a generic 500. + return handleWebApplicationException(waEx, requestUri); + } if (cause instanceof BaseAppException appEx) { // Delegate to specific BaseAppException handlers if (appEx instanceof PdfPasswordException) { diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/UserController.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/UserController.java index 961917a90a..117cd7f937 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/UserController.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/UserController.java @@ -25,6 +25,7 @@ import jakarta.ws.rs.core.Context; import jakarta.ws.rs.core.MediaType; import jakarta.ws.rs.core.Response; import jakarta.ws.rs.core.SecurityContext; +import jakarta.ws.rs.core.UriInfo; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; @@ -77,6 +78,15 @@ public class UserController { // method parameters. securityContext.getUserPrincipal() is null when unauthenticated. @Context SecurityContext securityContext; + // Spring's @RequestParam bound a value from EITHER the query string OR the form body. + // RESTEasy's + // @RestForm only reads the body, so admin/account endpoints invoked with query parameters (as + // the regression suite and some clients do) would otherwise see null. UriInfo lets the + // @RestForm + // params fall back to the query string, restoring the original union semantics. See + // formOrQuery / formOrQueryLong / formOrQueryBool. + @Context UriInfo uriInfo; + // TODO: Migration required - @PreAuthorize("!hasAuthority('ROLE_DEMO_USER')") is not a simple // role check, so it cannot be expressed via @RolesAllowed. Re-implement the DEMO_USER exclusion // as a runtime check against the current identity's roles (e.g. via SecurityIdentity). @@ -351,9 +361,11 @@ public class UserController { @jakarta.ws.rs.Path("/change-password") @Audited(type = AuditEventType.USER_PROFILE_UPDATE, level = AuditLevel.BASIC) public Response changePassword( - @RestForm(value = "currentPassword") String currentPassword, - @RestForm(value = "newPassword") String newPassword) + @RestForm(value = "currentPassword") String currentPasswordForm, + @RestForm(value = "newPassword") String newPasswordForm) throws SQLException, UnsupportedProviderException { + String currentPassword = formOrQuery(currentPasswordForm, "currentPassword"); + String newPassword = formOrQuery(newPasswordForm, "newPassword"); if (securityContext.getUserPrincipal() == null) { return Response.status(Response.Status.UNAUTHORIZED) .entity( @@ -423,15 +435,22 @@ public class UserController { @POST @jakarta.ws.rs.Path("/admin/saveUser") public Response saveUser( - @RestForm(value = "username") String username, - @RestForm(value = "password") String password, - @RestForm(value = "role") String role, - @RestForm(value = "teamId") Long teamId, - @RestForm(value = "authType") String authType, - @RestForm(value = "forceChange") boolean forceChange, - @RestForm(value = "forceMFA") boolean forceMFA) + @RestForm(value = "username") String usernameForm, + @RestForm(value = "password") String passwordForm, + @RestForm(value = "role") String roleForm, + @RestForm(value = "teamId") Long teamIdForm, + @RestForm(value = "authType") String authTypeForm, + @RestForm(value = "forceChange") Boolean forceChangeForm, + @RestForm(value = "forceMFA") Boolean forceMFAForm) throws IllegalArgumentException, SQLException, UnsupportedProviderException { - if (!userService.isUsernameValid(username)) { + String username = formOrQuery(usernameForm, "username"); + String password = formOrQuery(passwordForm, "password"); + String role = formOrQuery(roleForm, "role"); + Long teamId = formOrQueryLong(teamIdForm, "teamId"); + String authType = formOrQuery(authTypeForm, "authType"); + boolean forceChange = formOrQueryBool(forceChangeForm, "forceChange", false); + boolean forceMFA = formOrQueryBool(forceMFAForm, "forceMFA", false); + if (username == null || !userService.isUsernameValid(username)) { return Response.status(Response.Status.BAD_REQUEST) .entity( Map.of( @@ -671,10 +690,13 @@ public class UserController { @jakarta.ws.rs.Path("/admin/changeRole") @Transactional public Response changeRole( - @RestForm(value = "username") String username, - @RestForm(value = "role") String role, - @RestForm(value = "teamId") Long teamId) + @RestForm(value = "username") String usernameForm, + @RestForm(value = "role") String roleForm, + @RestForm(value = "teamId") Long teamIdForm) throws SQLException, UnsupportedProviderException { + String username = formOrQuery(usernameForm, "username"); + String role = formOrQuery(roleForm, "role"); + Long teamId = formOrQueryLong(teamIdForm, "teamId"); Optional userOpt = userService.findByUsernameIgnoreCase(username); if (!userOpt.isPresent()) { return Response.status(Response.Status.NOT_FOUND) @@ -743,14 +765,21 @@ public class UserController { @POST @jakarta.ws.rs.Path("/admin/changePasswordForUser") public Response changePasswordForUser( - @RestForm(value = "username") String username, - @RestForm(value = "newPassword") String newPassword, - @RestForm(value = "generateRandom") boolean generateRandom, - @RestForm(value = "sendEmail") boolean sendEmail, - @RestForm(value = "includePassword") boolean includePassword, - @RestForm(value = "forcePasswordChange") boolean forcePasswordChange, + @RestForm(value = "username") String usernameForm, + @RestForm(value = "newPassword") String newPasswordForm, + @RestForm(value = "generateRandom") Boolean generateRandomForm, + @RestForm(value = "sendEmail") Boolean sendEmailForm, + @RestForm(value = "includePassword") Boolean includePasswordForm, + @RestForm(value = "forcePasswordChange") Boolean forcePasswordChangeForm, @Context HttpServerRequest request) throws SQLException, UnsupportedProviderException, MessagingException { + String username = formOrQuery(usernameForm, "username"); + String newPassword = formOrQuery(newPasswordForm, "newPassword"); + boolean generateRandom = formOrQueryBool(generateRandomForm, "generateRandom", false); + boolean sendEmail = formOrQueryBool(sendEmailForm, "sendEmail", false); + boolean includePassword = formOrQueryBool(includePasswordForm, "includePassword", false); + boolean forcePasswordChange = + formOrQueryBool(forcePasswordChangeForm, "forcePasswordChange", false); Optional userOpt = userService.findByUsernameIgnoreCase(username); if (userOpt.isEmpty()) { return Response.status(Response.Status.NOT_FOUND) @@ -821,8 +850,10 @@ public class UserController { @POST @jakarta.ws.rs.Path("/admin/changeUserEnabled/{username}") public Response changeUserEnabled( - @PathParam("username") String username, @RestForm(value = "enabled") boolean enabled) + @PathParam("username") String username, + @RestForm(value = "enabled") Boolean enabledForm) throws SQLException, UnsupportedProviderException { + boolean enabled = formOrQueryBool(enabledForm, "enabled", false); Optional userOpt = userService.findByUsernameIgnoreCase(username); if (userOpt.isEmpty()) { return Response.status(Response.Status.NOT_FOUND) @@ -918,7 +949,8 @@ public class UserController { @jakarta.ws.rs.Path("/get-api-key") public Response getApiKey() { if (securityContext.getUserPrincipal() == null) { - return Response.status(Response.Status.FORBIDDEN) + // Unauthenticated -> 401 (Spring's auth entry point returned 401 here, not 403). + return Response.status(Response.Status.UNAUTHORIZED) .entity(Map.of("error", "User not authenticated.")) .build(); } @@ -938,7 +970,8 @@ public class UserController { @jakarta.ws.rs.Path("/update-api-key") public Response updateApiKey() { if (securityContext.getUserPrincipal() == null) { - return Response.status(Response.Status.FORBIDDEN) + // Unauthenticated -> 401 (Spring's auth entry point returned 401 here, not 403). + return Response.status(Response.Status.UNAUTHORIZED) .entity(Map.of("error", "User not authenticated.")) .build(); } @@ -1120,4 +1153,39 @@ public class UserController { user.getTeam() != null ? user.getTeam().getName() : null, user.isEnabled()); } + + // ─── @RequestParam-style binding (query OR form) ────────────────────────────────────────── + // These restore Spring's @RequestParam union behavior: prefer the value bound from the form + // body (@RestForm), falling back to the same-named query parameter when the body did not carry + // it. Keeps the frontend's FormData posts working while also accepting query-string callers. + + private String formOrQuery(String formValue, String name) { + if (formValue != null) { + return formValue; + } + return uriInfo != null ? uriInfo.getQueryParameters().getFirst(name) : null; + } + + private Long formOrQueryLong(Long formValue, String name) { + if (formValue != null) { + return formValue; + } + String raw = uriInfo != null ? uriInfo.getQueryParameters().getFirst(name) : null; + if (raw == null || raw.isBlank()) { + return null; + } + try { + return Long.valueOf(raw.trim()); + } catch (NumberFormatException ex) { + return null; + } + } + + private boolean formOrQueryBool(Boolean formValue, String name, boolean defaultValue) { + if (formValue != null) { + return formValue; + } + String raw = uriInfo != null ? uriInfo.getQueryParameters().getFirst(name) : null; + return raw != null ? Boolean.parseBoolean(raw.trim()) : defaultValue; + } } diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/service/UserService.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/service/UserService.java index 3bba3462b0..dbe69091e1 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/service/UserService.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/service/UserService.java @@ -414,11 +414,14 @@ public class UserService implements UserServiceInterface { databaseService.exportDatabase(); } + @Transactional public void changeRole(User user, String newRole) throws SQLException, UnsupportedProviderException { Authority userAuthority = this.findRole(user); userAuthority.setAuthority(newRole); - authorityRepository.persist(userAuthority); + // The authority was loaded in a prior request/transaction, so it is detached; Panache + // persist() rejects a detached entity. Re-attach via merge (see changeUserEnabled). + authorityRepository.getEntityManager().merge(userAuthority); databaseService.exportDatabase(); } @@ -426,7 +429,10 @@ public class UserService implements UserServiceInterface { public void changeUserEnabled(User user, Boolean enbeled) throws SQLException, UnsupportedProviderException { user.setEnabled(enbeled); - userRepository.persist(user); + // The user was loaded in a prior request/transaction, so it is detached here; Panache + // persist() rejects a detached entity ("Detached entity passed to persist"). Re-attach via + // merge to update it (same fix as changePassword / changeFirstUse). + userRepository.getEntityManager().merge(user); databaseService.exportDatabase(); } @@ -436,7 +442,8 @@ public class UserService implements UserServiceInterface { team = getDefaultTeam(); } user.setTeam(team); - userRepository.persist(user); + // Detached entity -> merge, not persist (see changeUserEnabled). + userRepository.getEntityManager().merge(user); databaseService.exportDatabase(); } diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/storage/controller/FileFolderPlacementController.java b/app/proprietary/src/main/java/stirling/software/proprietary/storage/controller/FileFolderPlacementController.java deleted file mode 100644 index e02f7ae21a..0000000000 --- a/app/proprietary/src/main/java/stirling/software/proprietary/storage/controller/FileFolderPlacementController.java +++ /dev/null @@ -1,91 +0,0 @@ -package stirling.software.proprietary.storage.controller; - -import java.util.List; -import java.util.UUID; - -import jakarta.enterprise.context.ApplicationScoped; -import jakarta.validation.Valid; -import jakarta.validation.constraints.NotNull; -import jakarta.validation.constraints.Size; -import jakarta.ws.rs.PATCH; -import jakarta.ws.rs.Path; -import jakarta.ws.rs.PathParam; -import jakarta.ws.rs.core.Response; - -import lombok.AllArgsConstructor; -import lombok.Data; -import lombok.NoArgsConstructor; -import lombok.RequiredArgsConstructor; - -import stirling.software.proprietary.storage.service.FolderService; - -/** - * Folder placement endpoints for existing stored files. Thin adapter: validates the request shape, - * delegates the transaction to {@link FolderService}, then maps the result onto the HTTP status. - * Authentication, storage-gate, ownership checks, and the bulk cap all live on the service (where - * {@code @Transactional} also lives) so the JDBC connection isn't held through JSON serialization. - */ -@ApplicationScoped -@Path("/api/v1/storage/files") -@RequiredArgsConstructor -public class FileFolderPlacementController { - - private static final int BULK_MOVE_MAX_FILES = 1000; - - private final FolderService folderService; - - /** Move a single file to a folder (or to root when folderId is null). */ - @PATCH - @Path("/{fileId}/folder") - public Response moveFileToFolder( - @PathParam("fileId") Long fileId, @Valid FolderPlacement body) { - folderService.moveFileToFolder(fileId, body.getFolderId()); - return Response.noContent().build(); - } - - /** - * Bulk move - fewer round-trips than calling the single endpoint N times. Returns 200 on full - * success, 207 (Multi-Status) when some files were skipped (typically because they don't belong - * to the caller). - */ - @PATCH - @Path("/folder") - public Response bulkMove(@Valid BulkMoveRequest body) { - FolderService.BulkMoveResult result = - folderService.bulkMoveFilesToFolder(body.getFolderId(), body.getFileIds()); - // 207 Multi-Status has no Response.Status constant; use the numeric code directly. - int status = result.skippedFileIds().isEmpty() ? Response.Status.OK.getStatusCode() : 207; - return Response.status(status) - .entity(new BulkMoveResponse(result.movedFileIds(), result.skippedFileIds())) - .build(); - } - - @Data - @NoArgsConstructor - @AllArgsConstructor - public static class FolderPlacement { - private UUID folderId; - } - - @Data - @NoArgsConstructor - @AllArgsConstructor - public static class BulkMoveRequest { - private UUID folderId; - - @NotNull - @Size( - min = 1, - max = BULK_MOVE_MAX_FILES, - message = "fileIds must contain between 1 and 1000 entries") - private List fileIds; - } - - @Data - @NoArgsConstructor - @AllArgsConstructor - public static class BulkMoveResponse { - private List movedFileIds; - private List skippedFileIds; - } -} diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/storage/controller/FileStorageController.java b/app/proprietary/src/main/java/stirling/software/proprietary/storage/controller/FileStorageController.java index 3e4aee9719..5ce7a2cef4 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/storage/controller/FileStorageController.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/storage/controller/FileStorageController.java @@ -7,6 +7,7 @@ import java.time.Duration; import java.util.List; import java.util.Locale; import java.util.Optional; +import java.util.UUID; import org.jboss.resteasy.reactive.RestForm; import org.jboss.resteasy.reactive.multipart.FileUpload; @@ -16,9 +17,13 @@ import io.swagger.v3.oas.annotations.tags.Tag; import jakarta.enterprise.context.ApplicationScoped; import jakarta.inject.Inject; +import jakarta.validation.Valid; +import jakarta.validation.constraints.NotNull; +import jakarta.validation.constraints.Size; import jakarta.ws.rs.Consumes; import jakarta.ws.rs.DELETE; import jakarta.ws.rs.GET; +import jakarta.ws.rs.PATCH; import jakarta.ws.rs.POST; import jakarta.ws.rs.PUT; import jakarta.ws.rs.Produces; @@ -29,6 +34,9 @@ import jakarta.ws.rs.core.MediaType; import jakarta.ws.rs.core.Response; import jakarta.ws.rs.core.StreamingOutput; +import lombok.AllArgsConstructor; +import lombok.Data; +import lombok.NoArgsConstructor; import lombok.extern.slf4j.Slf4j; import stirling.software.common.model.multipart.FileUploadMultipartFile; @@ -43,6 +51,7 @@ import stirling.software.proprietary.storage.model.api.ShareWithUserRequest; import stirling.software.proprietary.storage.model.api.StoredFileResponse; import stirling.software.proprietary.storage.provider.StorageProvider; import stirling.software.proprietary.storage.service.FileStorageService; +import stirling.software.proprietary.storage.service.FolderService; // IMPORTANT: this class also references java.nio-style paths indirectly; @jakarta.ws.rs.Path is // fully-qualified on the class/methods to avoid any clash with collaborator types. @@ -56,8 +65,11 @@ public class FileStorageController { private static final Duration SIGNED_URL_TTL = Duration.ofMinutes(5); + private static final int BULK_MOVE_MAX_FILES = 1000; + @Inject FileStorageService fileStorageService; @Inject StorageProvider storageProvider; + @Inject FolderService folderService; // TODO: Migration required - SecurityIdentity replaces Spring's Authentication. The // collaborator @@ -122,6 +134,41 @@ public class FileStorageController { return fileStorageService.getAccessibleFileResponse(user, fileId); } + // ─── File ↔ folder placement ────────────────────────────────────────────────────────────── + // These live here (rather than in a separate @Path("/api/v1/storage/files") resource) so a + // single JAX-RS resource owns the whole /api/v1/storage/files sub-tree. Splitting them across + // two resource classes made the more-specific class shadow this one, so POST /files (upload) + // resolved to a class with only @PATCH methods and returned 405. Authentication, the + // storage-gate, ownership checks and the bulk cap all live on FolderService (with + // @Transactional) + // so the JDBC connection isn't held through JSON serialization. + + /** Move a single file to a folder (or to root when folderId is null). */ + @PATCH + @jakarta.ws.rs.Path("/files/{fileId}/folder") + public Response moveFileToFolder( + @jakarta.ws.rs.PathParam("fileId") Long fileId, @Valid FolderPlacement body) { + folderService.moveFileToFolder(fileId, body.getFolderId()); + return Response.noContent().build(); + } + + /** + * Bulk move - fewer round-trips than calling the single endpoint N times. Returns 200 on full + * success, 207 (Multi-Status) when some files were skipped (typically because they don't belong + * to the caller). + */ + @PATCH + @jakarta.ws.rs.Path("/files/folder") + public Response bulkMove(@Valid BulkMoveRequest body) { + FolderService.BulkMoveResult result = + folderService.bulkMoveFilesToFolder(body.getFolderId(), body.getFileIds()); + // 207 Multi-Status has no Response.Status constant; use the numeric code directly. + int status = result.skippedFileIds().isEmpty() ? Response.Status.OK.getStatusCode() : 207; + return Response.status(status) + .entity(new BulkMoveResponse(result.movedFileIds(), result.skippedFileIds())) + .build(); + } + @GET @jakarta.ws.rs.Path("/files/{fileId}/download") public Response downloadFile( @@ -361,4 +408,33 @@ public class FileStorageController { return Optional.empty(); } } + + @Data + @NoArgsConstructor + @AllArgsConstructor + public static class FolderPlacement { + private UUID folderId; + } + + @Data + @NoArgsConstructor + @AllArgsConstructor + public static class BulkMoveRequest { + private UUID folderId; + + @NotNull + @Size( + min = 1, + max = BULK_MOVE_MAX_FILES, + message = "fileIds must contain between 1 and 1000 entries") + private List fileIds; + } + + @Data + @NoArgsConstructor + @AllArgsConstructor + public static class BulkMoveResponse { + private List movedFileIds; + private List skippedFileIds; + } } diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/storage/service/FileStorageService.java b/app/proprietary/src/main/java/stirling/software/proprietary/storage/service/FileStorageService.java index 96d234a65b..4469e1dfd0 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/storage/service/FileStorageService.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/storage/service/FileStorageService.java @@ -1,6 +1,7 @@ package stirling.software.proprietary.storage.service; import java.io.IOException; +import java.security.Principal; import java.time.LocalDateTime; import java.time.temporal.ChronoUnit; import java.util.Comparator; @@ -16,8 +17,11 @@ import java.util.UUID; import java.util.regex.Pattern; import java.util.stream.Collectors; +import io.quarkus.security.identity.SecurityIdentity; + import jakarta.enterprise.context.ApplicationScoped; import jakarta.enterprise.inject.Instance; +import jakarta.inject.Inject; import jakarta.mail.MessagingException; import jakarta.transaction.Transactional; import jakarta.ws.rs.WebApplicationException; @@ -30,7 +34,6 @@ import stirling.software.common.model.ApplicationProperties; import stirling.software.common.model.MultipartFile; import stirling.software.common.model.io.Resource; import stirling.software.common.security.Authentication; -import stirling.software.common.security.SecurityContextHolder; import stirling.software.proprietary.security.database.repository.UserRepository; import stirling.software.proprietary.security.model.User; import stirling.software.proprietary.security.service.EmailService; @@ -73,6 +76,12 @@ public class FileStorageService { private final Instance emailService; private final StorageCleanupEntryRepository storageCleanupEntryRepository; + // Field injection (not a constructor arg) so the @RequiredArgsConstructor signature stays + // stable + // for the unit test that builds this service directly. SecurityIdentity is the Quarkus + // replacement for Spring's SecurityContextHolder - see requireAuthenticatedUser. + @Inject SecurityIdentity securityIdentity; + public void ensureStorageEnabled() { if (!applicationProperties.getSecurity().isEnableLogin()) { throw new WebApplicationException( @@ -84,18 +93,18 @@ public class FileStorageService { } public User requireAuthenticatedUser() { - Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); - if (authentication == null - || !authentication.isAuthenticated() - || "anonymousUser".equals(authentication.getPrincipal())) { + // Spring's SecurityContextHolder is never populated under Quarkus; the authenticated + // principal is exposed via SecurityIdentity instead (a SecurityIdentityAugmentor attaches + // the + // User entity as the principal, the same wiring FolderService.requireAuthenticatedUser + // uses). + if (securityIdentity == null || securityIdentity.isAnonymous()) { throw new WebApplicationException("Not authenticated", Response.Status.UNAUTHORIZED); } - - Object principal = authentication.getPrincipal(); + Principal principal = securityIdentity.getPrincipal(); if (principal instanceof User user) { return user; } - throw new WebApplicationException( "Unsupported user principal", Response.Status.UNAUTHORIZED); } diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/storage/service/FolderService.java b/app/proprietary/src/main/java/stirling/software/proprietary/storage/service/FolderService.java index 0e81f94f63..6cfcfcc8a6 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/storage/service/FolderService.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/storage/service/FolderService.java @@ -58,9 +58,9 @@ public class FolderService { /** * Hard cap on bulk-move payload size, mirroring the request-validation cap on {@code - * FileFolderPlacementController.BulkMoveRequest.fileIds}. Re-asserted at the service layer - * because controller-level @Valid bounds aren't enforced when the service is called directly - * (e.g. by future internal callers or tests). + * FileStorageController.BulkMoveRequest.fileIds}. Re-asserted at the service layer because + * controller-level @Valid bounds aren't enforced when the service is called directly (e.g. by + * future internal callers or tests). */ private static final int BULK_MOVE_MAX_FILES = 1000;