From 5b4ae5590d8e64dc4a0bb29c346dcbeea1553871 Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+Frooodle@users.noreply.github.com.> Date: Wed, 2 Jul 2025 21:23:56 +0100 Subject: [PATCH 01/13] settings api --- .../api/AdminSettingsController.java | 332 ++++++++++++++++++ .../model/api/admin/SettingValueResponse.java | 20 ++ .../api/admin/UpdateSettingValueRequest.java | 13 + .../api/admin/UpdateSettingsRequest.java | 25 ++ 4 files changed, 390 insertions(+) create mode 100644 proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java create mode 100644 proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/SettingValueResponse.java create mode 100644 proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/UpdateSettingValueRequest.java create mode 100644 proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/UpdateSettingsRequest.java diff --git a/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java b/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java new file mode 100644 index 000000000..496820fac --- /dev/null +++ b/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java @@ -0,0 +1,332 @@ +package stirling.software.proprietary.security.controller.api; + +import java.io.IOException; +import java.util.Map; + +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; +import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.stereotype.Controller; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.PutMapping; +import org.springframework.web.bind.annotation.RequestBody; +import org.springframework.web.bind.annotation.RequestMapping; + +import io.swagger.v3.oas.annotations.Operation; +import io.swagger.v3.oas.annotations.responses.ApiResponse; +import io.swagger.v3.oas.annotations.responses.ApiResponses; +import io.swagger.v3.oas.annotations.tags.Tag; + +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; + +import stirling.software.common.configuration.InstallationPathConfig; +import stirling.software.common.model.ApplicationProperties; +import stirling.software.common.util.GeneralUtils; +import stirling.software.proprietary.security.model.api.admin.SettingValueResponse; +import stirling.software.proprietary.security.model.api.admin.UpdateSettingValueRequest; +import stirling.software.proprietary.security.model.api.admin.UpdateSettingsRequest; + +@Controller +@Tag(name = "Admin Settings", description = "Admin-only Settings Management APIs") +@RequestMapping("/api/v1/admin/settings") +@RequiredArgsConstructor +@PreAuthorize("hasRole('ROLE_ADMIN')") +@Slf4j +public class AdminSettingsController { + + private final ApplicationProperties applicationProperties; + + @GetMapping + @Operation( + summary = "Get all application settings", + description = "Retrieve all current application settings. Admin access required.") + @ApiResponses( + value = { + @ApiResponse(responseCode = "200", description = "Settings retrieved successfully"), + @ApiResponse( + responseCode = "403", + description = "Access denied - Admin role required") + }) + public ResponseEntity getSettings() { + log.debug("Admin requested all application settings"); + return ResponseEntity.ok(applicationProperties); + } + + @PutMapping + @Operation( + summary = "Update application settings (delta updates)", + description = + "Update specific application settings using dot notation keys. Only sends changed values. Changes take effect on restart. Admin access required.") + @ApiResponses( + value = { + @ApiResponse(responseCode = "200", description = "Settings updated successfully"), + @ApiResponse(responseCode = "400", description = "Invalid setting key or value"), + @ApiResponse( + responseCode = "403", + description = "Access denied - Admin role required"), + @ApiResponse( + responseCode = "500", + description = "Failed to save settings to configuration file") + }) + public ResponseEntity updateSettings(@RequestBody UpdateSettingsRequest request) { + try { + Map settings = request.getSettings(); + if (settings == null || settings.isEmpty()) { + return ResponseEntity.badRequest().body("No settings provided to update"); + } + + int updatedCount = 0; + for (Map.Entry entry : settings.entrySet()) { + String key = entry.getKey(); + Object value = entry.getValue(); + + log.info("Admin updating setting: {} = {}", key, value); + GeneralUtils.saveKeyToSettings(key, value); + updatedCount++; + } + + return ResponseEntity.ok( + String.format( + "Successfully updated %d setting(s). Changes will take effect on application restart.", + updatedCount)); + + } catch (IOException e) { + log.error("Failed to save settings to file: {}", e.getMessage(), e); + return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) + .body( + "Failed to save settings to configuration file at: " + + InstallationPathConfig.getSettingsPath() + + ". Error: " + + e.getMessage()); + + } catch (Exception e) { + log.error("Unexpected error while updating settings: {}", e.getMessage(), e); + return ResponseEntity.status(HttpStatus.BAD_REQUEST) + .body("Invalid setting key or value. Error: " + e.getMessage()); + } + } + + @GetMapping("/section/{sectionName}") + @Operation( + summary = "Get specific settings section", + description = + "Retrieve settings for a specific section (e.g., security, system, ui). Admin access required.") + @ApiResponses( + value = { + @ApiResponse( + responseCode = "200", + description = "Section settings retrieved successfully"), + @ApiResponse(responseCode = "400", description = "Invalid section name"), + @ApiResponse( + responseCode = "403", + description = "Access denied - Admin role required") + }) + public ResponseEntity getSettingsSection(@PathVariable String sectionName) { + try { + Object sectionData = getSectionData(sectionName); + if (sectionData == null) { + return ResponseEntity.badRequest() + .body( + "Invalid section name: " + + sectionName + + ". Valid sections: security, system, ui, endpoints, metrics, mail, premium, processExecutor, autoPipeline"); + } + log.debug("Admin requested settings section: {}", sectionName); + return ResponseEntity.ok(sectionData); + } catch (Exception e) { + log.error("Error retrieving section {}: {}", sectionName, e.getMessage(), e); + return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) + .body("Failed to retrieve section: " + e.getMessage()); + } + } + + @PutMapping("/section/{sectionName}") + @Operation( + summary = "Update specific settings section", + description = "Update all settings within a specific section. Admin access required.") + @ApiResponses( + value = { + @ApiResponse( + responseCode = "200", + description = "Section settings updated successfully"), + @ApiResponse(responseCode = "400", description = "Invalid section name or data"), + @ApiResponse( + responseCode = "403", + description = "Access denied - Admin role required"), + @ApiResponse(responseCode = "500", description = "Failed to save settings") + }) + public ResponseEntity updateSettingsSection( + @PathVariable String sectionName, @RequestBody Map sectionData) { + try { + if (sectionData == null || sectionData.isEmpty()) { + return ResponseEntity.badRequest().body("No section data provided to update"); + } + + if (!isValidSectionName(sectionName)) { + return ResponseEntity.badRequest() + .body( + "Invalid section name: " + + sectionName + + ". Valid sections: security, system, ui, endpoints, metrics, mail, premium, processExecutor, autoPipeline"); + } + + int updatedCount = 0; + for (Map.Entry entry : sectionData.entrySet()) { + String key = sectionName + "." + entry.getKey(); + Object value = entry.getValue(); + + log.info("Admin updating section setting: {} = {}", key, value); + GeneralUtils.saveKeyToSettings(key, value); + updatedCount++; + } + + return ResponseEntity.ok( + String.format( + "Successfully updated %d setting(s) in section '%s'. Changes will take effect on application restart.", + updatedCount, sectionName)); + + } catch (IOException e) { + log.error("Failed to save section settings to file: {}", e.getMessage(), e); + return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) + .body("Failed to save settings to configuration file: " + e.getMessage()); + } catch (Exception e) { + log.error("Unexpected error while updating section settings: {}", e.getMessage(), e); + return ResponseEntity.status(HttpStatus.BAD_REQUEST) + .body("Invalid section data. Error: " + e.getMessage()); + } + } + + @GetMapping("/key/{key}") + @Operation( + summary = "Get specific setting value", + description = + "Retrieve value for a specific setting key using dot notation. Admin access required.") + @ApiResponses( + value = { + @ApiResponse( + responseCode = "200", + description = "Setting value retrieved successfully"), + @ApiResponse(responseCode = "400", description = "Invalid setting key"), + @ApiResponse( + responseCode = "403", + description = "Access denied - Admin role required") + }) + public ResponseEntity getSettingValue(@PathVariable String key) { + try { + Object value = getSettingByKey(key); + if (value == null) { + return ResponseEntity.badRequest().body("Setting key not found: " + key); + } + log.debug("Admin requested setting: {}", key); + return ResponseEntity.ok(new SettingValueResponse(key, value)); + } catch (Exception e) { + log.error("Error retrieving setting {}: {}", key, e.getMessage(), e); + return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) + .body("Failed to retrieve setting: " + e.getMessage()); + } + } + + @PutMapping("/key/{key}") + @Operation( + summary = "Update specific setting value", + description = + "Update value for a specific setting key using dot notation. Admin access required.") + @ApiResponses( + value = { + @ApiResponse(responseCode = "200", description = "Setting updated successfully"), + @ApiResponse(responseCode = "400", description = "Invalid setting key or value"), + @ApiResponse( + responseCode = "403", + description = "Access denied - Admin role required"), + @ApiResponse(responseCode = "500", description = "Failed to save setting") + }) + public ResponseEntity updateSettingValue( + @PathVariable String key, @RequestBody UpdateSettingValueRequest request) { + try { + if (request.getValue() == null) { + return ResponseEntity.badRequest().body("Request body must contain 'value' field"); + } + + Object value = request.getValue(); + log.info("Admin updating single setting: {} = {}", key, value); + GeneralUtils.saveKeyToSettings(key, value); + + return ResponseEntity.ok( + String.format( + "Successfully updated setting '%s'. Changes will take effect on application restart.", + key)); + + } catch (IOException e) { + log.error("Failed to save setting to file: {}", e.getMessage(), e); + return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) + .body("Failed to save setting to configuration file: " + e.getMessage()); + } catch (Exception e) { + log.error("Unexpected error while updating setting: {}", e.getMessage(), e); + return ResponseEntity.status(HttpStatus.BAD_REQUEST) + .body("Invalid setting key or value. Error: " + e.getMessage()); + } + } + + private Object getSectionData(String sectionName) { + return switch (sectionName.toLowerCase()) { + case "security" -> applicationProperties.getSecurity(); + case "system" -> applicationProperties.getSystem(); + case "ui" -> applicationProperties.getUi(); + case "endpoints" -> applicationProperties.getEndpoints(); + case "metrics" -> applicationProperties.getMetrics(); + case "mail" -> applicationProperties.getMail(); + case "premium" -> applicationProperties.getPremium(); + case "processexecutor" -> applicationProperties.getProcessExecutor(); + case "autopipeline" -> applicationProperties.getAutoPipeline(); + case "legal" -> applicationProperties.getLegal(); + default -> null; + }; + } + + private boolean isValidSectionName(String sectionName) { + return getSectionData(sectionName) != null; + } + + private Object getSettingByKey(String key) { + String[] parts = key.split("\\.", 2); + if (parts.length < 2) { + return null; + } + + String sectionName = parts[0]; + String propertyPath = parts[1]; + Object section = getSectionData(sectionName); + + if (section == null) { + return null; + } + + try { + return getNestedProperty(section, propertyPath); + } catch (Exception e) { + log.warn("Failed to get nested property {}: {}", key, e.getMessage()); + return null; + } + } + + private Object getNestedProperty(Object obj, String propertyPath) throws Exception { + if (obj == null) { + return null; + } + + String[] parts = propertyPath.split("\\.", 2); + String currentProperty = parts[0]; + + java.lang.reflect.Field field = obj.getClass().getDeclaredField(currentProperty); + field.setAccessible(true); + Object value = field.get(obj); + + if (parts.length == 1) { + return value; + } else { + return getNestedProperty(value, parts[1]); + } + } +} diff --git a/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/SettingValueResponse.java b/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/SettingValueResponse.java new file mode 100644 index 000000000..f13c5ffba --- /dev/null +++ b/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/SettingValueResponse.java @@ -0,0 +1,20 @@ +package stirling.software.proprietary.security.model.api.admin; + +import io.swagger.v3.oas.annotations.media.Schema; + +import lombok.AllArgsConstructor; +import lombok.Data; +import lombok.NoArgsConstructor; + +@Data +@AllArgsConstructor +@NoArgsConstructor +@Schema(description = "Response object containing a setting key and its value") +public class SettingValueResponse { + + @Schema(description = "The setting key in dot notation", example = "system.enableAnalytics") + private String key; + + @Schema(description = "The current value of the setting", example = "true") + private Object value; +} diff --git a/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/UpdateSettingValueRequest.java b/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/UpdateSettingValueRequest.java new file mode 100644 index 000000000..a49171989 --- /dev/null +++ b/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/UpdateSettingValueRequest.java @@ -0,0 +1,13 @@ +package stirling.software.proprietary.security.model.api.admin; + +import io.swagger.v3.oas.annotations.media.Schema; + +import lombok.Data; + +@Data +@Schema(description = "Request object for updating a single setting value") +public class UpdateSettingValueRequest { + + @Schema(description = "The new value for the setting", example = "true") + private Object value; +} diff --git a/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/UpdateSettingsRequest.java b/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/UpdateSettingsRequest.java new file mode 100644 index 000000000..85616f0b3 --- /dev/null +++ b/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/UpdateSettingsRequest.java @@ -0,0 +1,25 @@ +package stirling.software.proprietary.security.model.api.admin; + +import java.util.Map; + +import io.swagger.v3.oas.annotations.media.Schema; + +import lombok.Data; + +@Data +@Schema( + description = + "Request object for delta updates to application settings. Only include the settings you want to change. Uses dot notation for nested properties (e.g., 'system.enableAnalytics', 'ui.appName')") +public class UpdateSettingsRequest { + + @Schema( + description = + "Map of setting keys to their new values. Only include changed settings (delta updates). Keys use dot notation for nested properties.", + example = + "{\n" + + " \"system.enableAnalytics\": true,\n" + + " \"ui.appName\": \"My Custom PDF Tool\",\n" + + " \"security.enableLogin\": false\n" + + "}") + private Map settings; +} From 40d2a9015c5388be14e70c985b64918b6c27b458 Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+Frooodle@users.noreply.github.com.> Date: Mon, 21 Jul 2025 15:57:01 +0100 Subject: [PATCH 02/13] reduce logging --- .../api/AdminSettingsController.java | 42 ++++++++++--------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java index 496820fac..5fe1b9176 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java @@ -12,6 +12,7 @@ import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.PutMapping; import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.util.HtmlUtils; import io.swagger.v3.oas.annotations.Operation; import io.swagger.v3.oas.annotations.responses.ApiResponse; @@ -36,6 +37,11 @@ import stirling.software.proprietary.security.model.api.admin.UpdateSettingsRequ @Slf4j public class AdminSettingsController { + private static final java.util.Set VALID_SECTIONS = java.util.Set.of( + "security", "system", "ui", "endpoints", "metrics", "mail", + "premium", "processExecutor", "autoPipeline", "legal" + ); + private final ApplicationProperties applicationProperties; @GetMapping @@ -95,16 +101,12 @@ public class AdminSettingsController { } catch (IOException e) { log.error("Failed to save settings to file: {}", e.getMessage(), e); return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) - .body( - "Failed to save settings to configuration file at: " - + InstallationPathConfig.getSettingsPath() - + ". Error: " - + e.getMessage()); + .body("Failed to save settings to configuration file."); } catch (Exception e) { log.error("Unexpected error while updating settings: {}", e.getMessage(), e); return ResponseEntity.status(HttpStatus.BAD_REQUEST) - .body("Invalid setting key or value. Error: " + e.getMessage()); + .body("Invalid setting key or value."); } } @@ -130,15 +132,15 @@ public class AdminSettingsController { return ResponseEntity.badRequest() .body( "Invalid section name: " - + sectionName - + ". Valid sections: security, system, ui, endpoints, metrics, mail, premium, processExecutor, autoPipeline"); + + HtmlUtils.htmlEscape(sectionName) + + ". Valid sections: security, system, ui, endpoints, metrics, mail, premium, processExecutor, autoPipeline, legal"); } log.debug("Admin requested settings section: {}", sectionName); return ResponseEntity.ok(sectionData); } catch (Exception e) { log.error("Error retrieving section {}: {}", sectionName, e.getMessage(), e); return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) - .body("Failed to retrieve section: " + e.getMessage()); + .body("Failed to retrieve section."); } } @@ -168,8 +170,8 @@ public class AdminSettingsController { return ResponseEntity.badRequest() .body( "Invalid section name: " - + sectionName - + ". Valid sections: security, system, ui, endpoints, metrics, mail, premium, processExecutor, autoPipeline"); + + HtmlUtils.htmlEscape(sectionName) + + ". Valid sections: security, system, ui, endpoints, metrics, mail, premium, processExecutor, autoPipeline, legal"); } int updatedCount = 0; @@ -182,19 +184,20 @@ public class AdminSettingsController { updatedCount++; } + String escapedSectionName = HtmlUtils.htmlEscape(sectionName); return ResponseEntity.ok( String.format( "Successfully updated %d setting(s) in section '%s'. Changes will take effect on application restart.", - updatedCount, sectionName)); + updatedCount, escapedSectionName)); } catch (IOException e) { log.error("Failed to save section settings to file: {}", e.getMessage(), e); return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) - .body("Failed to save settings to configuration file: " + e.getMessage()); + .body("Failed to save settings to configuration file."); } catch (Exception e) { log.error("Unexpected error while updating section settings: {}", e.getMessage(), e); return ResponseEntity.status(HttpStatus.BAD_REQUEST) - .body("Invalid section data. Error: " + e.getMessage()); + .body("Invalid section data."); } } @@ -217,14 +220,14 @@ public class AdminSettingsController { try { Object value = getSettingByKey(key); if (value == null) { - return ResponseEntity.badRequest().body("Setting key not found: " + key); + return ResponseEntity.badRequest().body("Setting key not found: " + HtmlUtils.htmlEscape(key)); } log.debug("Admin requested setting: {}", key); return ResponseEntity.ok(new SettingValueResponse(key, value)); } catch (Exception e) { log.error("Error retrieving setting {}: {}", key, e.getMessage(), e); return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) - .body("Failed to retrieve setting: " + e.getMessage()); + .body("Failed to retrieve setting."); } } @@ -253,19 +256,20 @@ public class AdminSettingsController { log.info("Admin updating single setting: {} = {}", key, value); GeneralUtils.saveKeyToSettings(key, value); + String escapedKey = HtmlUtils.htmlEscape(key); return ResponseEntity.ok( String.format( "Successfully updated setting '%s'. Changes will take effect on application restart.", - key)); + escapedKey)); } catch (IOException e) { log.error("Failed to save setting to file: {}", e.getMessage(), e); return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) - .body("Failed to save setting to configuration file: " + e.getMessage()); + .body("Failed to save setting to configuration file."); } catch (Exception e) { log.error("Unexpected error while updating setting: {}", e.getMessage(), e); return ResponseEntity.status(HttpStatus.BAD_REQUEST) - .body("Invalid setting key or value. Error: " + e.getMessage()); + .body("Invalid setting key or value."); } } From af4e20c9711c27abde066beb794d2b9c4ee1929d Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+Frooodle@users.noreply.github.com.> Date: Mon, 21 Jul 2025 16:17:49 +0100 Subject: [PATCH 03/13] add @Valid --- .../controller/api/AdminSettingsController.java | 15 +++++---------- .../api/admin/UpdateSettingValueRequest.java | 5 ++++- .../model/api/admin/UpdateSettingsRequest.java | 8 +++++++- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java index 5fe1b9176..78910f8b8 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java @@ -14,6 +14,8 @@ import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.util.HtmlUtils; +import jakarta.validation.Valid; + import io.swagger.v3.oas.annotations.Operation; import io.swagger.v3.oas.annotations.responses.ApiResponse; import io.swagger.v3.oas.annotations.responses.ApiResponses; @@ -76,12 +78,9 @@ public class AdminSettingsController { responseCode = "500", description = "Failed to save settings to configuration file") }) - public ResponseEntity updateSettings(@RequestBody UpdateSettingsRequest request) { + public ResponseEntity updateSettings(@Valid @RequestBody UpdateSettingsRequest request) { try { Map settings = request.getSettings(); - if (settings == null || settings.isEmpty()) { - return ResponseEntity.badRequest().body("No settings provided to update"); - } int updatedCount = 0; for (Map.Entry entry : settings.entrySet()) { @@ -160,7 +159,7 @@ public class AdminSettingsController { @ApiResponse(responseCode = "500", description = "Failed to save settings") }) public ResponseEntity updateSettingsSection( - @PathVariable String sectionName, @RequestBody Map sectionData) { + @PathVariable String sectionName, @Valid @RequestBody Map sectionData) { try { if (sectionData == null || sectionData.isEmpty()) { return ResponseEntity.badRequest().body("No section data provided to update"); @@ -246,12 +245,8 @@ public class AdminSettingsController { @ApiResponse(responseCode = "500", description = "Failed to save setting") }) public ResponseEntity updateSettingValue( - @PathVariable String key, @RequestBody UpdateSettingValueRequest request) { + @PathVariable String key, @Valid @RequestBody UpdateSettingValueRequest request) { try { - if (request.getValue() == null) { - return ResponseEntity.badRequest().body("Request body must contain 'value' field"); - } - Object value = request.getValue(); log.info("Admin updating single setting: {} = {}", key, value); GeneralUtils.saveKeyToSettings(key, value); diff --git a/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/UpdateSettingValueRequest.java b/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/UpdateSettingValueRequest.java index a49171989..c1e0506d4 100644 --- a/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/UpdateSettingValueRequest.java +++ b/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/UpdateSettingValueRequest.java @@ -1,5 +1,7 @@ package stirling.software.proprietary.security.model.api.admin; +import jakarta.validation.constraints.NotNull; + import io.swagger.v3.oas.annotations.media.Schema; import lombok.Data; @@ -8,6 +10,7 @@ import lombok.Data; @Schema(description = "Request object for updating a single setting value") public class UpdateSettingValueRequest { - @Schema(description = "The new value for the setting", example = "true") + @NotNull(message = "Setting value cannot be null") + @Schema(description = "The new value for the setting", example = "true", required = true) private Object value; } diff --git a/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/UpdateSettingsRequest.java b/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/UpdateSettingsRequest.java index 85616f0b3..15dc2b6e7 100644 --- a/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/UpdateSettingsRequest.java +++ b/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/UpdateSettingsRequest.java @@ -2,6 +2,9 @@ package stirling.software.proprietary.security.model.api.admin; import java.util.Map; +import jakarta.validation.constraints.NotEmpty; +import jakarta.validation.constraints.NotNull; + import io.swagger.v3.oas.annotations.media.Schema; import lombok.Data; @@ -12,6 +15,8 @@ import lombok.Data; "Request object for delta updates to application settings. Only include the settings you want to change. Uses dot notation for nested properties (e.g., 'system.enableAnalytics', 'ui.appName')") public class UpdateSettingsRequest { + @NotNull(message = "Settings map cannot be null") + @NotEmpty(message = "Settings map cannot be empty") @Schema( description = "Map of setting keys to their new values. Only include changed settings (delta updates). Keys use dot notation for nested properties.", @@ -20,6 +25,7 @@ public class UpdateSettingsRequest { + " \"system.enableAnalytics\": true,\n" + " \"ui.appName\": \"My Custom PDF Tool\",\n" + " \"security.enableLogin\": false\n" - + "}") + + "}", + required = true) private Map settings; } From b72deaebf05371ad12ee3eefcddc8721f16c8369 Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+Frooodle@users.noreply.github.com.> Date: Mon, 21 Jul 2025 17:08:00 +0100 Subject: [PATCH 04/13] fixes --- .../api/AdminSettingsController.java | 30 ++++++++++++------- .../model/api/admin/SettingValueResponse.java | 22 ++++++++++++++ .../api/admin/UpdateSettingValueRequest.java | 16 ++++++++++ .../api/admin/UpdateSettingsRequest.java | 23 ++++++++++++++ 4 files changed, 80 insertions(+), 11 deletions(-) create mode 100644 app/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/SettingValueResponse.java create mode 100644 app/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/UpdateSettingValueRequest.java create mode 100644 app/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/UpdateSettingsRequest.java diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java index 78910f8b8..6ef89a1c9 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java @@ -14,17 +14,16 @@ import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.util.HtmlUtils; -import jakarta.validation.Valid; - import io.swagger.v3.oas.annotations.Operation; import io.swagger.v3.oas.annotations.responses.ApiResponse; import io.swagger.v3.oas.annotations.responses.ApiResponses; import io.swagger.v3.oas.annotations.tags.Tag; +import jakarta.validation.Valid; + import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; -import stirling.software.common.configuration.InstallationPathConfig; import stirling.software.common.model.ApplicationProperties; import stirling.software.common.util.GeneralUtils; import stirling.software.proprietary.security.model.api.admin.SettingValueResponse; @@ -39,10 +38,18 @@ import stirling.software.proprietary.security.model.api.admin.UpdateSettingsRequ @Slf4j public class AdminSettingsController { - private static final java.util.Set VALID_SECTIONS = java.util.Set.of( - "security", "system", "ui", "endpoints", "metrics", "mail", - "premium", "processExecutor", "autoPipeline", "legal" - ); + private static final java.util.Set VALID_SECTIONS = + java.util.Set.of( + "security", + "system", + "ui", + "endpoints", + "metrics", + "mail", + "premium", + "processExecutor", + "autoPipeline", + "legal"); private final ApplicationProperties applicationProperties; @@ -78,7 +85,8 @@ public class AdminSettingsController { responseCode = "500", description = "Failed to save settings to configuration file") }) - public ResponseEntity updateSettings(@Valid @RequestBody UpdateSettingsRequest request) { + public ResponseEntity updateSettings( + @Valid @RequestBody UpdateSettingsRequest request) { try { Map settings = request.getSettings(); @@ -195,8 +203,7 @@ public class AdminSettingsController { .body("Failed to save settings to configuration file."); } catch (Exception e) { log.error("Unexpected error while updating section settings: {}", e.getMessage(), e); - return ResponseEntity.status(HttpStatus.BAD_REQUEST) - .body("Invalid section data."); + return ResponseEntity.status(HttpStatus.BAD_REQUEST).body("Invalid section data."); } } @@ -219,7 +226,8 @@ public class AdminSettingsController { try { Object value = getSettingByKey(key); if (value == null) { - return ResponseEntity.badRequest().body("Setting key not found: " + HtmlUtils.htmlEscape(key)); + return ResponseEntity.badRequest() + .body("Setting key not found: " + HtmlUtils.htmlEscape(key)); } log.debug("Admin requested setting: {}", key); return ResponseEntity.ok(new SettingValueResponse(key, value)); diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/SettingValueResponse.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/SettingValueResponse.java new file mode 100644 index 000000000..1436a2291 --- /dev/null +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/SettingValueResponse.java @@ -0,0 +1,22 @@ +package stirling.software.proprietary.security.model.api.admin; + +import io.swagger.v3.oas.annotations.media.Schema; + +import lombok.AllArgsConstructor; +import lombok.Data; +import lombok.NoArgsConstructor; + +@Data +@NoArgsConstructor +@AllArgsConstructor +@Schema(description = "Response containing a setting key and its current value") +public class SettingValueResponse { + + @Schema( + description = "The setting key in dot notation (e.g., 'system.enableAnalytics')", + example = "system.enableAnalytics") + private String key; + + @Schema(description = "The current value of the setting", example = "true") + private Object value; +} diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/UpdateSettingValueRequest.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/UpdateSettingValueRequest.java new file mode 100644 index 000000000..76c18898f --- /dev/null +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/UpdateSettingValueRequest.java @@ -0,0 +1,16 @@ +package stirling.software.proprietary.security.model.api.admin; + +import io.swagger.v3.oas.annotations.media.Schema; + +import jakarta.validation.constraints.NotNull; + +import lombok.Data; + +@Data +@Schema(description = "Request to update a single setting value") +public class UpdateSettingValueRequest { + + @NotNull + @Schema(description = "The new value for the setting", example = "false") + private Object value; +} diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/UpdateSettingsRequest.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/UpdateSettingsRequest.java new file mode 100644 index 000000000..7e1342fab --- /dev/null +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/UpdateSettingsRequest.java @@ -0,0 +1,23 @@ +package stirling.software.proprietary.security.model.api.admin; + +import java.util.Map; + +import io.swagger.v3.oas.annotations.media.Schema; + +import jakarta.validation.constraints.NotEmpty; +import jakarta.validation.constraints.NotNull; + +import lombok.Data; + +@Data +@Schema(description = "Request to update multiple application settings using delta updates") +public class UpdateSettingsRequest { + + @NotNull + @NotEmpty + @Schema( + description = + "Map of setting keys to their new values using dot notation. Only changed values need to be included for delta updates.", + example = "{\"system.enableAnalytics\": false, \"ui.appName\": \"My PDF Tool\"}") + private Map settings; +} From fc52d14ab1702ef8c897dcf5d34fe19f9d12be7a Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+Frooodle@users.noreply.github.com.> Date: Fri, 25 Jul 2025 15:23:56 +0100 Subject: [PATCH 05/13] fixes --- .../software/common/util/GeneralUtils.java | 202 +++++++++++++++++- .../api/AdminSettingsController.java | 180 ++++++++++++---- 2 files changed, 341 insertions(+), 41 deletions(-) diff --git a/app/common/src/main/java/stirling/software/common/util/GeneralUtils.java b/app/common/src/main/java/stirling/software/common/util/GeneralUtils.java index a164faec2..6c1d7ae1a 100644 --- a/app/common/src/main/java/stirling/software/common/util/GeneralUtils.java +++ b/app/common/src/main/java/stirling/software/common/util/GeneralUtils.java @@ -5,6 +5,8 @@ import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; import java.net.*; +import java.nio.channels.FileChannel; +import java.nio.channels.FileLock; import java.nio.charset.StandardCharsets; import java.nio.file.*; import java.nio.file.attribute.BasicFileAttributes; @@ -15,6 +17,8 @@ import java.util.Enumeration; import java.util.List; import java.util.Locale; import java.util.UUID; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.ReentrantReadWriteLock; import org.springframework.core.io.ClassPathResource; import org.springframework.core.io.Resource; @@ -37,6 +41,25 @@ public class GeneralUtils { private static final List DEFAULT_VALID_SCRIPTS = List.of("png_to_webp.py", "split_photos.py"); + // Concurrency control for settings file operations + private static final ReentrantReadWriteLock settingsLock = + new ReentrantReadWriteLock(true); // fair locking + private static volatile String lastSettingsHash = null; + + // Lock timeout configuration + private static final long LOCK_TIMEOUT_SECONDS = 30; // Maximum time to wait for locks + private static final long FILE_LOCK_TIMEOUT_MS = 5000; // File lock timeout + + // Initialize settings hash on first access + static { + try { + lastSettingsHash = calculateSettingsHash(); + } catch (Exception e) { + log.warn("Could not initialize settings hash: {}", e.getMessage()); + lastSettingsHash = ""; + } + } + public static File convertMultipartFileToFile(MultipartFile multipartFile) throws IOException { String customTempDir = System.getenv("STIRLING_TEMPFILES_DIRECTORY"); if (customTempDir == null || customTempDir.isEmpty()) { @@ -397,12 +420,183 @@ public class GeneralUtils { * Internal Implementation Details * *------------------------------------------------------------------------*/ + /** + * Thread-safe method to save a key-value pair to settings file with concurrency control. + * Prevents race conditions and data corruption when multiple threads/admins modify settings. + * + * @param key The setting key in dot notation (e.g., "security.enableCSRF") + * @param newValue The new value to set + * @throws IOException If file operations fail + * @throws IllegalStateException If settings file was modified by another process + */ public static void saveKeyToSettings(String key, Object newValue) throws IOException { - String[] keyArray = key.split("\\."); + // Use timeout to prevent infinite blocking + boolean lockAcquired = false; + try { + lockAcquired = settingsLock.writeLock().tryLock(LOCK_TIMEOUT_SECONDS, TimeUnit.SECONDS); + if (!lockAcquired) { + throw new IOException( + String.format( + "Could not acquire write lock for setting '%s' within %d seconds. " + + "Another admin operation may be in progress or the system may be under heavy load.", + key, LOCK_TIMEOUT_SECONDS)); + } + Path settingsPath = Paths.get(InstallationPathConfig.getSettingsPath()); + + // Attempt file locking with timeout and retry logic + FileLock fileLock = null; + long startTime = System.currentTimeMillis(); + + while (fileLock == null + && (System.currentTimeMillis() - startTime) < FILE_LOCK_TIMEOUT_MS) { + try (FileChannel channel = + FileChannel.open( + settingsPath, + StandardOpenOption.READ, + StandardOpenOption.WRITE, + StandardOpenOption.CREATE)) { + + // Try non-blocking lock first + fileLock = channel.tryLock(); + + if (fileLock != null) { + try { + // Validate that we can actually read/write to detect stale locks + if (!Files.isWritable(settingsPath)) { + throw new IOException( + "Settings file is not writable - permissions issue"); + } + + // Check for concurrent modifications + String currentHash = calculateSettingsHash(); + if (lastSettingsHash != null && !lastSettingsHash.equals(currentHash)) { + log.info( + "Settings file was modified externally for key: {} - updating hash", + key); + lastSettingsHash = currentHash; + } + + // Perform the actual update + String[] keyArray = key.split("\\."); + YamlHelper settingsYaml = new YamlHelper(settingsPath); + settingsYaml.updateValue(Arrays.asList(keyArray), newValue); + settingsYaml.saveOverride(settingsPath); + + // Update hash after successful write + lastSettingsHash = calculateSettingsHash(); + + log.debug("Successfully updated setting: {} = {}", key, newValue); + return; // Success - exit method + + } finally { + // Ensure file lock is always released + if (fileLock != null && fileLock.isValid()) { + try { + fileLock.release(); + } catch (IOException e) { + log.warn( + "Failed to release file lock for setting {}: {}", + key, + e.getMessage()); + } + } + } + } else { + // Lock not available, wait briefly before retry + Thread.sleep(100); + } + + } catch (IOException e) { + if (fileLock != null && fileLock.isValid()) { + try { + fileLock.release(); + } catch (IOException releaseError) { + log.warn( + "Failed to release file lock after error: {}", + releaseError.getMessage()); + } + } + throw e; + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new IOException("Interrupted while waiting for file lock", e); + } + } + + // If we get here, we couldn't acquire the file lock within timeout + throw new IOException( + String.format( + "Could not acquire file lock for setting '%s' within %d ms. " + + "The settings file may be locked by another process or there may be file system issues.", + key, FILE_LOCK_TIMEOUT_MS)); + + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new IOException("Interrupted while waiting for settings lock", e); + } catch (Exception e) { + log.error("Unexpected error updating setting {}: {}", key, e.getMessage(), e); + if (e instanceof IOException) { + throw (IOException) e; + } + throw new IOException("Failed to update settings: " + e.getMessage(), e); + } finally { + if (lockAcquired) { + settingsLock.writeLock().unlock(); + } + } + } + + /** + * Calculates MD5 hash of the settings file for change detection + * + * @return Hash string, or empty string if file doesn't exist + */ + private static String calculateSettingsHash() throws Exception { Path settingsPath = Paths.get(InstallationPathConfig.getSettingsPath()); - YamlHelper settingsYaml = new YamlHelper(settingsPath); - settingsYaml.updateValue(Arrays.asList(keyArray), newValue); - settingsYaml.saveOverride(settingsPath); + if (!Files.exists(settingsPath)) { + return ""; + } + + byte[] fileBytes = Files.readAllBytes(settingsPath); + MessageDigest md = MessageDigest.getInstance("MD5"); + byte[] hashBytes = md.digest(fileBytes); + + StringBuilder sb = new StringBuilder(); + for (byte b : hashBytes) { + sb.append(String.format("%02x", b)); + } + return sb.toString(); + } + + /** + * Thread-safe method to read settings values with proper locking and timeout + * + * @return YamlHelper instance for reading + * @throws IOException If timeout occurs or file operations fail + */ + public static YamlHelper getSettingsReader() throws IOException { + boolean lockAcquired = false; + try { + lockAcquired = settingsLock.readLock().tryLock(LOCK_TIMEOUT_SECONDS, TimeUnit.SECONDS); + if (!lockAcquired) { + throw new IOException( + String.format( + "Could not acquire read lock for settings within %d seconds. " + + "System may be under heavy load or there may be a deadlock.", + LOCK_TIMEOUT_SECONDS)); + } + + Path settingsPath = Paths.get(InstallationPathConfig.getSettingsPath()); + return new YamlHelper(settingsPath); + + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new IOException("Interrupted while waiting for settings read lock", e); + } finally { + if (lockAcquired) { + settingsLock.readLock().unlock(); + } + } } public static String generateMachineFingerprint() { diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java index 6ef89a1c9..b87378055 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java @@ -2,6 +2,7 @@ package stirling.software.proprietary.security.controller.api; import java.io.IOException; import java.util.Map; +import java.util.regex.Pattern; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; @@ -14,6 +15,8 @@ import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.util.HtmlUtils; +import com.fasterxml.jackson.databind.ObjectMapper; + import io.swagger.v3.oas.annotations.Operation; import io.swagger.v3.oas.annotations.responses.ApiResponse; import io.swagger.v3.oas.annotations.responses.ApiResponses; @@ -38,20 +41,8 @@ import stirling.software.proprietary.security.model.api.admin.UpdateSettingsRequ @Slf4j public class AdminSettingsController { - private static final java.util.Set VALID_SECTIONS = - java.util.Set.of( - "security", - "system", - "ui", - "endpoints", - "metrics", - "mail", - "premium", - "processExecutor", - "autoPipeline", - "legal"); - private final ApplicationProperties applicationProperties; + private final ObjectMapper objectMapper; @GetMapping @Operation( @@ -89,12 +80,20 @@ public class AdminSettingsController { @Valid @RequestBody UpdateSettingsRequest request) { try { Map settings = request.getSettings(); + if (settings == null || settings.isEmpty()) { + return ResponseEntity.badRequest().body("No settings provided to update"); + } int updatedCount = 0; for (Map.Entry entry : settings.entrySet()) { String key = entry.getKey(); Object value = entry.getValue(); + if (!isValidSettingKey(key)) { + return ResponseEntity.badRequest() + .body("Invalid setting key format: " + HtmlUtils.htmlEscape(key)); + } + log.info("Admin updating setting: {} = {}", key, value); GeneralUtils.saveKeyToSettings(key, value); updatedCount++; @@ -110,10 +109,14 @@ public class AdminSettingsController { return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) .body("Failed to save settings to configuration file."); + } catch (IllegalArgumentException e) { + log.error("Invalid setting key or value: {}", e.getMessage(), e); + return ResponseEntity.status(HttpStatus.BAD_REQUEST) + .body("Invalid setting key or value: " + e.getMessage()); } catch (Exception e) { log.error("Unexpected error while updating settings: {}", e.getMessage(), e); - return ResponseEntity.status(HttpStatus.BAD_REQUEST) - .body("Invalid setting key or value."); + return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) + .body("Internal server error occurred while updating settings."); } } @@ -140,10 +143,15 @@ public class AdminSettingsController { .body( "Invalid section name: " + HtmlUtils.htmlEscape(sectionName) - + ". Valid sections: security, system, ui, endpoints, metrics, mail, premium, processExecutor, autoPipeline, legal"); + + ". Valid sections: " + + String.join(", ", VALID_SECTION_NAMES)); } log.debug("Admin requested settings section: {}", sectionName); return ResponseEntity.ok(sectionData); + } catch (IllegalArgumentException e) { + log.error("Invalid section name {}: {}", sectionName, e.getMessage(), e); + return ResponseEntity.status(HttpStatus.BAD_REQUEST) + .body("Invalid section name: " + HtmlUtils.htmlEscape(sectionName)); } catch (Exception e) { log.error("Error retrieving section {}: {}", sectionName, e.getMessage(), e); return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) @@ -178,16 +186,23 @@ public class AdminSettingsController { .body( "Invalid section name: " + HtmlUtils.htmlEscape(sectionName) - + ". Valid sections: security, system, ui, endpoints, metrics, mail, premium, processExecutor, autoPipeline, legal"); + + ". Valid sections: " + + String.join(", ", VALID_SECTION_NAMES)); } int updatedCount = 0; for (Map.Entry entry : sectionData.entrySet()) { - String key = sectionName + "." + entry.getKey(); + String propertyKey = entry.getKey(); + String fullKey = sectionName + "." + propertyKey; Object value = entry.getValue(); - log.info("Admin updating section setting: {} = {}", key, value); - GeneralUtils.saveKeyToSettings(key, value); + if (!isValidSettingKey(fullKey)) { + return ResponseEntity.badRequest() + .body("Invalid setting key format: " + HtmlUtils.htmlEscape(fullKey)); + } + + log.info("Admin updating section setting: {} = {}", fullKey, value); + GeneralUtils.saveKeyToSettings(fullKey, value); updatedCount++; } @@ -201,9 +216,14 @@ public class AdminSettingsController { log.error("Failed to save section settings to file: {}", e.getMessage(), e); return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) .body("Failed to save settings to configuration file."); + } catch (IllegalArgumentException e) { + log.error("Invalid section data: {}", e.getMessage(), e); + return ResponseEntity.status(HttpStatus.BAD_REQUEST) + .body("Invalid section data: " + e.getMessage()); } catch (Exception e) { log.error("Unexpected error while updating section settings: {}", e.getMessage(), e); - return ResponseEntity.status(HttpStatus.BAD_REQUEST).body("Invalid section data."); + return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) + .body("Internal server error occurred while updating section settings."); } } @@ -224,6 +244,11 @@ public class AdminSettingsController { }) public ResponseEntity getSettingValue(@PathVariable String key) { try { + if (!isValidSettingKey(key)) { + return ResponseEntity.badRequest() + .body("Invalid setting key format: " + HtmlUtils.htmlEscape(key)); + } + Object value = getSettingByKey(key); if (value == null) { return ResponseEntity.badRequest() @@ -231,6 +256,10 @@ public class AdminSettingsController { } log.debug("Admin requested setting: {}", key); return ResponseEntity.ok(new SettingValueResponse(key, value)); + } catch (IllegalArgumentException e) { + log.error("Invalid setting key {}: {}", key, e.getMessage(), e); + return ResponseEntity.status(HttpStatus.BAD_REQUEST) + .body("Invalid setting key: " + HtmlUtils.htmlEscape(key)); } catch (Exception e) { log.error("Error retrieving setting {}: {}", key, e.getMessage(), e); return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) @@ -255,6 +284,11 @@ public class AdminSettingsController { public ResponseEntity updateSettingValue( @PathVariable String key, @Valid @RequestBody UpdateSettingValueRequest request) { try { + if (!isValidSettingKey(key)) { + return ResponseEntity.badRequest() + .body("Invalid setting key format: " + HtmlUtils.htmlEscape(key)); + } + Object value = request.getValue(); log.info("Admin updating single setting: {} = {}", key, value); GeneralUtils.saveKeyToSettings(key, value); @@ -269,14 +303,22 @@ public class AdminSettingsController { log.error("Failed to save setting to file: {}", e.getMessage(), e); return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) .body("Failed to save setting to configuration file."); + } catch (IllegalArgumentException e) { + log.error("Invalid setting key or value: {}", e.getMessage(), e); + return ResponseEntity.status(HttpStatus.BAD_REQUEST) + .body("Invalid setting key or value: " + e.getMessage()); } catch (Exception e) { log.error("Unexpected error while updating setting: {}", e.getMessage(), e); - return ResponseEntity.status(HttpStatus.BAD_REQUEST) - .body("Invalid setting key or value."); + return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) + .body("Internal server error occurred while updating setting."); } } private Object getSectionData(String sectionName) { + if (sectionName == null || sectionName.trim().isEmpty()) { + return null; + } + return switch (sectionName.toLowerCase()) { case "security" -> applicationProperties.getSecurity(); case "system" -> applicationProperties.getSystem(); @@ -285,8 +327,8 @@ public class AdminSettingsController { case "metrics" -> applicationProperties.getMetrics(); case "mail" -> applicationProperties.getMail(); case "premium" -> applicationProperties.getPremium(); - case "processexecutor" -> applicationProperties.getProcessExecutor(); - case "autopipeline" -> applicationProperties.getAutoPipeline(); + case "processexecutor", "processExecutor" -> applicationProperties.getProcessExecutor(); + case "autopipeline", "autoPipeline" -> applicationProperties.getAutoPipeline(); case "legal" -> applicationProperties.getLegal(); default -> null; }; @@ -296,7 +338,54 @@ public class AdminSettingsController { return getSectionData(sectionName) != null; } + private static final java.util.Set VALID_SECTION_NAMES = + java.util.Set.of( + "security", + "system", + "ui", + "endpoints", + "metrics", + "mail", + "premium", + "processExecutor", + "processexecutor", + "autoPipeline", + "autopipeline", + "legal"); + + // Pattern to validate safe property paths - only alphanumeric, dots, and underscores + private static final Pattern SAFE_KEY_PATTERN = Pattern.compile("^[a-zA-Z0-9._]+$"); + private static final int MAX_NESTING_DEPTH = 10; + + private boolean isValidSettingKey(String key) { + if (key == null || key.trim().isEmpty()) { + return false; + } + + // Check against pattern to prevent injection attacks + if (!SAFE_KEY_PATTERN.matcher(key).matches()) { + return false; + } + + // Prevent excessive nesting depth + String[] parts = key.split("\\."); + if (parts.length > MAX_NESTING_DEPTH) { + return false; + } + + // Ensure first part is a valid section name + if (parts.length > 0 && !VALID_SECTION_NAMES.contains(parts[0].toLowerCase())) { + return false; + } + + return true; + } + private Object getSettingByKey(String key) { + if (key == null || key.trim().isEmpty()) { + return null; + } + String[] parts = key.split("\\.", 2); if (parts.length < 2) { return null; @@ -311,29 +400,46 @@ public class AdminSettingsController { } try { - return getNestedProperty(section, propertyPath); - } catch (Exception e) { + return getNestedProperty(section, propertyPath, 0); + } catch (NoSuchFieldException | IllegalAccessException e) { log.warn("Failed to get nested property {}: {}", key, e.getMessage()); return null; } } - private Object getNestedProperty(Object obj, String propertyPath) throws Exception { + private Object getNestedProperty(Object obj, String propertyPath, int depth) + throws NoSuchFieldException, IllegalAccessException { if (obj == null) { return null; } - String[] parts = propertyPath.split("\\.", 2); - String currentProperty = parts[0]; + // Prevent excessive recursion depth + if (depth > MAX_NESTING_DEPTH) { + throw new IllegalAccessException("Maximum nesting depth exceeded"); + } - java.lang.reflect.Field field = obj.getClass().getDeclaredField(currentProperty); - field.setAccessible(true); - Object value = field.get(obj); + try { + // Use Jackson ObjectMapper for safer property access + @SuppressWarnings("unchecked") + Map objectMap = objectMapper.convertValue(obj, Map.class); - if (parts.length == 1) { - return value; - } else { - return getNestedProperty(value, parts[1]); + String[] parts = propertyPath.split("\\.", 2); + String currentProperty = parts[0]; + + if (!objectMap.containsKey(currentProperty)) { + throw new NoSuchFieldException("Property not found: " + currentProperty); + } + + Object value = objectMap.get(currentProperty); + + if (parts.length == 1) { + return value; + } else { + return getNestedProperty(value, parts[1], depth + 1); + } + } catch (IllegalArgumentException e) { + // If Jackson fails, the property doesn't exist or isn't accessible + throw new NoSuchFieldException("Property not accessible: " + propertyPath); } } } From 4f19da5395054e4f25899d559e9ca39b63ee8006 Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+Frooodle@users.noreply.github.com.> Date: Fri, 25 Jul 2025 15:30:24 +0100 Subject: [PATCH 06/13] remove dups --- .../model/api/admin/SettingValueResponse.java | 20 ------------ .../api/admin/UpdateSettingValueRequest.java | 16 ---------- .../api/admin/UpdateSettingsRequest.java | 31 ------------------- 3 files changed, 67 deletions(-) delete mode 100644 proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/SettingValueResponse.java delete mode 100644 proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/UpdateSettingValueRequest.java delete mode 100644 proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/UpdateSettingsRequest.java diff --git a/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/SettingValueResponse.java b/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/SettingValueResponse.java deleted file mode 100644 index f13c5ffba..000000000 --- a/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/SettingValueResponse.java +++ /dev/null @@ -1,20 +0,0 @@ -package stirling.software.proprietary.security.model.api.admin; - -import io.swagger.v3.oas.annotations.media.Schema; - -import lombok.AllArgsConstructor; -import lombok.Data; -import lombok.NoArgsConstructor; - -@Data -@AllArgsConstructor -@NoArgsConstructor -@Schema(description = "Response object containing a setting key and its value") -public class SettingValueResponse { - - @Schema(description = "The setting key in dot notation", example = "system.enableAnalytics") - private String key; - - @Schema(description = "The current value of the setting", example = "true") - private Object value; -} diff --git a/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/UpdateSettingValueRequest.java b/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/UpdateSettingValueRequest.java deleted file mode 100644 index c1e0506d4..000000000 --- a/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/UpdateSettingValueRequest.java +++ /dev/null @@ -1,16 +0,0 @@ -package stirling.software.proprietary.security.model.api.admin; - -import jakarta.validation.constraints.NotNull; - -import io.swagger.v3.oas.annotations.media.Schema; - -import lombok.Data; - -@Data -@Schema(description = "Request object for updating a single setting value") -public class UpdateSettingValueRequest { - - @NotNull(message = "Setting value cannot be null") - @Schema(description = "The new value for the setting", example = "true", required = true) - private Object value; -} diff --git a/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/UpdateSettingsRequest.java b/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/UpdateSettingsRequest.java deleted file mode 100644 index 15dc2b6e7..000000000 --- a/proprietary/src/main/java/stirling/software/proprietary/security/model/api/admin/UpdateSettingsRequest.java +++ /dev/null @@ -1,31 +0,0 @@ -package stirling.software.proprietary.security.model.api.admin; - -import java.util.Map; - -import jakarta.validation.constraints.NotEmpty; -import jakarta.validation.constraints.NotNull; - -import io.swagger.v3.oas.annotations.media.Schema; - -import lombok.Data; - -@Data -@Schema( - description = - "Request object for delta updates to application settings. Only include the settings you want to change. Uses dot notation for nested properties (e.g., 'system.enableAnalytics', 'ui.appName')") -public class UpdateSettingsRequest { - - @NotNull(message = "Settings map cannot be null") - @NotEmpty(message = "Settings map cannot be empty") - @Schema( - description = - "Map of setting keys to their new values. Only include changed settings (delta updates). Keys use dot notation for nested properties.", - example = - "{\n" - + " \"system.enableAnalytics\": true,\n" - + " \"ui.appName\": \"My Custom PDF Tool\",\n" - + " \"security.enableLogin\": false\n" - + "}", - required = true) - private Map settings; -} From b543a72ee9f32fd3e7185a02f6b21799e388d127 Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+Frooodle@users.noreply.github.com.> Date: Fri, 25 Jul 2025 15:34:08 +0100 Subject: [PATCH 07/13] reduce logs clientside --- .../api/AdminSettingsController.java | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java index b87378055..50c816ea8 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java @@ -106,17 +106,15 @@ public class AdminSettingsController { } catch (IOException e) { log.error("Failed to save settings to file: {}", e.getMessage(), e); - return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) - .body("Failed to save settings to configuration file."); + return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(GENERIC_FILE_ERROR); } catch (IllegalArgumentException e) { log.error("Invalid setting key or value: {}", e.getMessage(), e); - return ResponseEntity.status(HttpStatus.BAD_REQUEST) - .body("Invalid setting key or value: " + e.getMessage()); + return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(GENERIC_INVALID_SETTING); } catch (Exception e) { log.error("Unexpected error while updating settings: {}", e.getMessage(), e); return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) - .body("Internal server error occurred while updating settings."); + .body(GENERIC_SERVER_ERROR); } } @@ -214,16 +212,14 @@ public class AdminSettingsController { } catch (IOException e) { log.error("Failed to save section settings to file: {}", e.getMessage(), e); - return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) - .body("Failed to save settings to configuration file."); + return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(GENERIC_FILE_ERROR); } catch (IllegalArgumentException e) { log.error("Invalid section data: {}", e.getMessage(), e); - return ResponseEntity.status(HttpStatus.BAD_REQUEST) - .body("Invalid section data: " + e.getMessage()); + return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(GENERIC_INVALID_SECTION); } catch (Exception e) { log.error("Unexpected error while updating section settings: {}", e.getMessage(), e); return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) - .body("Internal server error occurred while updating section settings."); + .body(GENERIC_SERVER_ERROR); } } @@ -301,16 +297,14 @@ public class AdminSettingsController { } catch (IOException e) { log.error("Failed to save setting to file: {}", e.getMessage(), e); - return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) - .body("Failed to save setting to configuration file."); + return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(GENERIC_FILE_ERROR); } catch (IllegalArgumentException e) { log.error("Invalid setting key or value: {}", e.getMessage(), e); - return ResponseEntity.status(HttpStatus.BAD_REQUEST) - .body("Invalid setting key or value: " + e.getMessage()); + return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(GENERIC_INVALID_SETTING); } catch (Exception e) { log.error("Unexpected error while updating setting: {}", e.getMessage(), e); return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) - .body("Internal server error occurred while updating setting."); + .body(GENERIC_SERVER_ERROR); } } @@ -357,6 +351,13 @@ public class AdminSettingsController { private static final Pattern SAFE_KEY_PATTERN = Pattern.compile("^[a-zA-Z0-9._]+$"); private static final int MAX_NESTING_DEPTH = 10; + // Security: Generic error messages to prevent information disclosure + private static final String GENERIC_INVALID_SETTING = "Invalid setting key or value."; + private static final String GENERIC_INVALID_SECTION = "Invalid section data provided."; + private static final String GENERIC_SERVER_ERROR = "Internal server error occurred."; + private static final String GENERIC_FILE_ERROR = + "Failed to save settings to configuration file."; + private boolean isValidSettingKey(String key) { if (key == null || key.trim().isEmpty()) { return false; From f18d3c46ebbd8002c8a2e7d7c0b4fe6f816d4240 Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+Frooodle@users.noreply.github.com.> Date: Fri, 25 Jul 2025 16:49:20 +0100 Subject: [PATCH 08/13] further test --- .../common/model/ApplicationProperties.java | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java b/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java index e4edf2baa..920bbed98 100644 --- a/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java +++ b/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java @@ -25,6 +25,8 @@ import org.springframework.core.io.Resource; import org.springframework.core.io.support.EncodedResource; import org.springframework.stereotype.Component; +import com.fasterxml.jackson.annotation.JsonIgnore; + import lombok.Data; import lombok.Getter; import lombok.Setter; @@ -176,6 +178,7 @@ public class ApplicationProperties { @ToString.Exclude private String privateKey; @ToString.Exclude private String spCert; + @JsonIgnore public InputStream getIdpMetadataUri() throws IOException { if (idpMetadataUri.startsWith("classpath:")) { return new ClassPathResource(idpMetadataUri.substring("classpath".length())) @@ -192,6 +195,7 @@ public class ApplicationProperties { } } + @JsonIgnore public Resource getSpCert() { if (spCert == null) return null; if (spCert.startsWith("classpath:")) { @@ -201,6 +205,7 @@ public class ApplicationProperties { } } + @JsonIgnore public Resource getIdpCert() { if (idpCert == null) return null; if (idpCert.startsWith("classpath:")) { @@ -210,6 +215,7 @@ public class ApplicationProperties { } } + @JsonIgnore public Resource getPrivateKey() { if (privateKey.startsWith("classpath:")) { return new ClassPathResource(privateKey.substring("classpath:".length())); @@ -329,12 +335,14 @@ public class ApplicationProperties { private boolean startupCleanup = true; private boolean cleanupSystemTemp = false; + @JsonIgnore public String getBaseTmpDir() { return baseTmpDir != null && !baseTmpDir.isEmpty() ? baseTmpDir : java.lang.System.getProperty("java.io.tmpdir") + "/stirling-pdf"; } + @JsonIgnore public String getLibreofficeDir() { return libreofficeDir != null && !libreofficeDir.isEmpty() ? libreofficeDir @@ -548,42 +556,52 @@ public class ApplicationProperties { private int ghostscriptSessionLimit; private int ocrMyPdfSessionLimit; + @JsonIgnore public int getQpdfSessionLimit() { return qpdfSessionLimit > 0 ? qpdfSessionLimit : 2; } + @JsonIgnore public int getTesseractSessionLimit() { return tesseractSessionLimit > 0 ? tesseractSessionLimit : 1; } + @JsonIgnore public int getLibreOfficeSessionLimit() { return libreOfficeSessionLimit > 0 ? libreOfficeSessionLimit : 1; } + @JsonIgnore public int getPdfToHtmlSessionLimit() { return pdfToHtmlSessionLimit > 0 ? pdfToHtmlSessionLimit : 1; } + @JsonIgnore public int getPythonOpenCvSessionLimit() { return pythonOpenCvSessionLimit > 0 ? pythonOpenCvSessionLimit : 8; } + @JsonIgnore public int getWeasyPrintSessionLimit() { return weasyPrintSessionLimit > 0 ? weasyPrintSessionLimit : 16; } + @JsonIgnore public int getInstallAppSessionLimit() { return installAppSessionLimit > 0 ? installAppSessionLimit : 1; } + @JsonIgnore public int getCalibreSessionLimit() { return calibreSessionLimit > 0 ? calibreSessionLimit : 1; } + @JsonIgnore public int getGhostscriptSessionLimit() { return ghostscriptSessionLimit > 0 ? ghostscriptSessionLimit : 8; } + @JsonIgnore public int getOcrMyPdfSessionLimit() { return ocrMyPdfSessionLimit > 0 ? ocrMyPdfSessionLimit : 2; } @@ -602,42 +620,52 @@ public class ApplicationProperties { private long ghostscriptTimeoutMinutes; private long ocrMyPdfTimeoutMinutes; + @JsonIgnore public long getTesseractTimeoutMinutes() { return tesseractTimeoutMinutes > 0 ? tesseractTimeoutMinutes : 30; } + @JsonIgnore public long getQpdfTimeoutMinutes() { return qpdfTimeoutMinutes > 0 ? qpdfTimeoutMinutes : 30; } + @JsonIgnore public long getLibreOfficeTimeoutMinutes() { return libreOfficeTimeoutMinutes > 0 ? libreOfficeTimeoutMinutes : 30; } + @JsonIgnore public long getPdfToHtmlTimeoutMinutes() { return pdfToHtmlTimeoutMinutes > 0 ? pdfToHtmlTimeoutMinutes : 20; } + @JsonIgnore public long getPythonOpenCvTimeoutMinutes() { return pythonOpenCvTimeoutMinutes > 0 ? pythonOpenCvTimeoutMinutes : 30; } + @JsonIgnore public long getWeasyPrintTimeoutMinutes() { return weasyPrintTimeoutMinutes > 0 ? weasyPrintTimeoutMinutes : 30; } + @JsonIgnore public long getInstallAppTimeoutMinutes() { return installAppTimeoutMinutes > 0 ? installAppTimeoutMinutes : 60; } + @JsonIgnore public long getCalibreTimeoutMinutes() { return calibreTimeoutMinutes > 0 ? calibreTimeoutMinutes : 30; } + @JsonIgnore public long getGhostscriptTimeoutMinutes() { return ghostscriptTimeoutMinutes > 0 ? ghostscriptTimeoutMinutes : 30; } + @JsonIgnore public long getOcrMyPdfTimeoutMinutes() { return ocrMyPdfTimeoutMinutes > 0 ? ocrMyPdfTimeoutMinutes : 30; } From 3baacd4cd9ea333ae6df0457f32e8a0b60ec82f5 Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+Frooodle@users.noreply.github.com.> Date: Fri, 25 Jul 2025 16:59:20 +0100 Subject: [PATCH 09/13] tests --- .../stirling/software/common/model/ApplicationProperties.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java b/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java index 920bbed98..178ff4782 100644 --- a/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java +++ b/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java @@ -25,6 +25,7 @@ import org.springframework.core.io.Resource; import org.springframework.core.io.support.EncodedResource; import org.springframework.stereotype.Component; +import com.fasterxml.jackson.annotation.JsonAutoDetect; import com.fasterxml.jackson.annotation.JsonIgnore; import lombok.Data; @@ -544,6 +545,7 @@ public class ApplicationProperties { private TimeoutMinutes timeoutMinutes = new TimeoutMinutes(); @Data + @JsonAutoDetect(fieldVisibility = JsonAutoDetect.Visibility.ANY, getterVisibility = JsonAutoDetect.Visibility.NONE) public static class SessionLimit { private int libreOfficeSessionLimit; private int pdfToHtmlSessionLimit; @@ -608,6 +610,7 @@ public class ApplicationProperties { } @Data + @JsonAutoDetect(fieldVisibility = JsonAutoDetect.Visibility.ANY, getterVisibility = JsonAutoDetect.Visibility.NONE) public static class TimeoutMinutes { private long libreOfficeTimeoutMinutes; private long pdfToHtmlTimeoutMinutes; From 4ce1c906c048cf7e0ec32ba106a5c1aaf4037f4b Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+Frooodle@users.noreply.github.com.> Date: Fri, 25 Jul 2025 17:19:55 +0100 Subject: [PATCH 10/13] changes --- .../common/model/ApplicationProperties.java | 44 ++++++++----------- 1 file changed, 19 insertions(+), 25 deletions(-) diff --git a/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java b/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java index 178ff4782..e8eca30bd 100644 --- a/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java +++ b/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java @@ -27,6 +27,7 @@ import org.springframework.stereotype.Component; import com.fasterxml.jackson.annotation.JsonAutoDetect; import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.annotation.JsonProperty; import lombok.Data; import lombok.Getter; @@ -171,13 +172,20 @@ public class ApplicationProperties { private Boolean autoCreateUser = false; private Boolean blockRegistration = false; private String registrationId = "stirling"; - @ToString.Exclude private String idpMetadataUri; + @ToString.Exclude + @JsonProperty("idpMetadataUri") + private String idpMetadataUri; private String idpSingleLogoutUrl; private String idpSingleLoginUrl; private String idpIssuer; + @JsonProperty("idpCert") private String idpCert; - @ToString.Exclude private String privateKey; - @ToString.Exclude private String spCert; + @ToString.Exclude + @JsonProperty("privateKey") + private String privateKey; + @ToString.Exclude + @JsonProperty("spCert") + private String spCert; @JsonIgnore public InputStream getIdpMetadataUri() throws IOException { @@ -327,7 +335,9 @@ public class ApplicationProperties { @Data public static class TempFileManagement { + @JsonProperty("baseTmpDir") private String baseTmpDir = ""; + @JsonProperty("libreofficeDir") private String libreofficeDir = ""; private String systemTempDir = ""; private String prefix = "stirling-pdf-"; @@ -545,7 +555,6 @@ public class ApplicationProperties { private TimeoutMinutes timeoutMinutes = new TimeoutMinutes(); @Data - @JsonAutoDetect(fieldVisibility = JsonAutoDetect.Visibility.ANY, getterVisibility = JsonAutoDetect.Visibility.NONE) public static class SessionLimit { private int libreOfficeSessionLimit; private int pdfToHtmlSessionLimit; @@ -558,117 +567,102 @@ public class ApplicationProperties { private int ghostscriptSessionLimit; private int ocrMyPdfSessionLimit; - @JsonIgnore public int getQpdfSessionLimit() { return qpdfSessionLimit > 0 ? qpdfSessionLimit : 2; } - @JsonIgnore public int getTesseractSessionLimit() { return tesseractSessionLimit > 0 ? tesseractSessionLimit : 1; } - @JsonIgnore public int getLibreOfficeSessionLimit() { return libreOfficeSessionLimit > 0 ? libreOfficeSessionLimit : 1; } - @JsonIgnore public int getPdfToHtmlSessionLimit() { return pdfToHtmlSessionLimit > 0 ? pdfToHtmlSessionLimit : 1; } - @JsonIgnore public int getPythonOpenCvSessionLimit() { return pythonOpenCvSessionLimit > 0 ? pythonOpenCvSessionLimit : 8; } - @JsonIgnore public int getWeasyPrintSessionLimit() { return weasyPrintSessionLimit > 0 ? weasyPrintSessionLimit : 16; } - @JsonIgnore public int getInstallAppSessionLimit() { return installAppSessionLimit > 0 ? installAppSessionLimit : 1; } - @JsonIgnore public int getCalibreSessionLimit() { return calibreSessionLimit > 0 ? calibreSessionLimit : 1; } - @JsonIgnore public int getGhostscriptSessionLimit() { return ghostscriptSessionLimit > 0 ? ghostscriptSessionLimit : 8; } - @JsonIgnore public int getOcrMyPdfSessionLimit() { return ocrMyPdfSessionLimit > 0 ? ocrMyPdfSessionLimit : 2; } } @Data - @JsonAutoDetect(fieldVisibility = JsonAutoDetect.Visibility.ANY, getterVisibility = JsonAutoDetect.Visibility.NONE) public static class TimeoutMinutes { + @JsonProperty("libreOfficetimeoutMinutes") private long libreOfficeTimeoutMinutes; + @JsonProperty("pdfToHtmltimeoutMinutes") private long pdfToHtmlTimeoutMinutes; + @JsonProperty("pythonOpenCvtimeoutMinutes") private long pythonOpenCvTimeoutMinutes; + @JsonProperty("weasyPrinttimeoutMinutes") private long weasyPrintTimeoutMinutes; + @JsonProperty("installApptimeoutMinutes") private long installAppTimeoutMinutes; + @JsonProperty("calibretimeoutMinutes") private long calibreTimeoutMinutes; private long tesseractTimeoutMinutes; private long qpdfTimeoutMinutes; private long ghostscriptTimeoutMinutes; private long ocrMyPdfTimeoutMinutes; - @JsonIgnore public long getTesseractTimeoutMinutes() { return tesseractTimeoutMinutes > 0 ? tesseractTimeoutMinutes : 30; } - @JsonIgnore public long getQpdfTimeoutMinutes() { return qpdfTimeoutMinutes > 0 ? qpdfTimeoutMinutes : 30; } - @JsonIgnore public long getLibreOfficeTimeoutMinutes() { return libreOfficeTimeoutMinutes > 0 ? libreOfficeTimeoutMinutes : 30; } - @JsonIgnore public long getPdfToHtmlTimeoutMinutes() { return pdfToHtmlTimeoutMinutes > 0 ? pdfToHtmlTimeoutMinutes : 20; } - @JsonIgnore public long getPythonOpenCvTimeoutMinutes() { return pythonOpenCvTimeoutMinutes > 0 ? pythonOpenCvTimeoutMinutes : 30; } - @JsonIgnore public long getWeasyPrintTimeoutMinutes() { return weasyPrintTimeoutMinutes > 0 ? weasyPrintTimeoutMinutes : 30; } - @JsonIgnore public long getInstallAppTimeoutMinutes() { return installAppTimeoutMinutes > 0 ? installAppTimeoutMinutes : 60; } - @JsonIgnore public long getCalibreTimeoutMinutes() { return calibreTimeoutMinutes > 0 ? calibreTimeoutMinutes : 30; } - @JsonIgnore public long getGhostscriptTimeoutMinutes() { return ghostscriptTimeoutMinutes > 0 ? ghostscriptTimeoutMinutes : 30; } - @JsonIgnore public long getOcrMyPdfTimeoutMinutes() { return ocrMyPdfTimeoutMinutes > 0 ? ocrMyPdfTimeoutMinutes : 30; } From 8132f230ef826cec701b17a30b88a38f4948c76e Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+Frooodle@users.noreply.github.com.> Date: Mon, 28 Jul 2025 11:34:52 +0100 Subject: [PATCH 11/13] Improve settings file lock handling and add admin file endpoint --- .../common/model/ApplicationProperties.java | 14 +- .../software/common/util/GeneralUtils.java | 383 +++++++++++++++--- .../api/AdminSettingsController.java | 46 +++ 3 files changed, 388 insertions(+), 55 deletions(-) diff --git a/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java b/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java index 14076cbc5..a2e4a0b30 100644 --- a/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java +++ b/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java @@ -25,7 +25,6 @@ import org.springframework.core.io.Resource; import org.springframework.core.io.support.EncodedResource; import org.springframework.stereotype.Component; -import com.fasterxml.jackson.annotation.JsonAutoDetect; import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonProperty; @@ -172,17 +171,22 @@ public class ApplicationProperties { private Boolean autoCreateUser = false; private Boolean blockRegistration = false; private String registrationId = "stirling"; + @ToString.Exclude @JsonProperty("idpMetadataUri") private String idpMetadataUri; + private String idpSingleLogoutUrl; private String idpSingleLoginUrl; private String idpIssuer; + @JsonProperty("idpCert") private String idpCert; + @ToString.Exclude @JsonProperty("privateKey") private String privateKey; + @ToString.Exclude @JsonProperty("spCert") private String spCert; @@ -338,8 +342,10 @@ public class ApplicationProperties { public static class TempFileManagement { @JsonProperty("baseTmpDir") private String baseTmpDir = ""; + @JsonProperty("libreofficeDir") private String libreofficeDir = ""; + private String systemTempDir = ""; private String prefix = "stirling-pdf-"; private long maxAgeHours = 24; @@ -632,16 +638,22 @@ public class ApplicationProperties { public static class TimeoutMinutes { @JsonProperty("libreOfficetimeoutMinutes") private long libreOfficeTimeoutMinutes; + @JsonProperty("pdfToHtmltimeoutMinutes") private long pdfToHtmlTimeoutMinutes; + @JsonProperty("pythonOpenCvtimeoutMinutes") private long pythonOpenCvTimeoutMinutes; + @JsonProperty("weasyPrinttimeoutMinutes") private long weasyPrintTimeoutMinutes; + @JsonProperty("installApptimeoutMinutes") private long installAppTimeoutMinutes; + @JsonProperty("calibretimeoutMinutes") private long calibreTimeoutMinutes; + private long tesseractTimeoutMinutes; private long qpdfTimeoutMinutes; private long ghostscriptTimeoutMinutes; diff --git a/app/common/src/main/java/stirling/software/common/util/GeneralUtils.java b/app/common/src/main/java/stirling/software/common/util/GeneralUtils.java index 6c1d7ae1a..e96e4e5cf 100644 --- a/app/common/src/main/java/stirling/software/common/util/GeneralUtils.java +++ b/app/common/src/main/java/stirling/software/common/util/GeneralUtils.java @@ -4,9 +4,11 @@ import java.io.File; import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; +import java.lang.management.ManagementFactory; import java.net.*; import java.nio.channels.FileChannel; import java.nio.channels.FileLock; +import java.nio.channels.OverlappingFileLockException; import java.nio.charset.StandardCharsets; import java.nio.file.*; import java.nio.file.attribute.BasicFileAttributes; @@ -17,6 +19,7 @@ import java.util.Enumeration; import java.util.List; import java.util.Locale; import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.ReentrantReadWriteLock; @@ -48,7 +51,15 @@ public class GeneralUtils { // Lock timeout configuration private static final long LOCK_TIMEOUT_SECONDS = 30; // Maximum time to wait for locks - private static final long FILE_LOCK_TIMEOUT_MS = 5000; // File lock timeout + private static final long FILE_LOCK_TIMEOUT_MS = 1000; // File lock timeout + + // Track active file locks for diagnostics + private static final ConcurrentHashMap activeLocks = new ConcurrentHashMap<>(); + + // Configuration flag for force bypass mode + private static final boolean FORCE_BYPASS_EXTERNAL_LOCKS = + Boolean.parseBoolean( + System.getProperty("stirling.settings.force-bypass-locks", "true")); // Initialize settings hash on first access static { @@ -443,23 +454,53 @@ public class GeneralUtils { } Path settingsPath = Paths.get(InstallationPathConfig.getSettingsPath()); - // Attempt file locking with timeout and retry logic - FileLock fileLock = null; + // Get current thread and process info for diagnostics + String currentThread = Thread.currentThread().getName(); + String currentProcess = ManagementFactory.getRuntimeMXBean().getName(); + String lockAttemptInfo = + String.format( + "Thread:%s Process:%s Setting:%s", currentThread, currentProcess, key); + + log.debug( + "Attempting to acquire file lock for setting '{}' from {}", + key, + lockAttemptInfo); + + // Check what locks are currently active + if (!activeLocks.isEmpty()) { + log.debug("Active locks detected: {}", activeLocks); + } + + // Attempt file locking with proper retry logic long startTime = System.currentTimeMillis(); + int retryCount = 0; + final int maxRetries = (int) (FILE_LOCK_TIMEOUT_MS / 100); // 100ms intervals - while (fileLock == null - && (System.currentTimeMillis() - startTime) < FILE_LOCK_TIMEOUT_MS) { - try (FileChannel channel = - FileChannel.open( - settingsPath, - StandardOpenOption.READ, - StandardOpenOption.WRITE, - StandardOpenOption.CREATE)) { + while ((System.currentTimeMillis() - startTime) < FILE_LOCK_TIMEOUT_MS) { + FileChannel channel = null; + FileLock fileLock = null; - // Try non-blocking lock first + try { + // Open channel and keep it open during lock attempts + channel = + FileChannel.open( + settingsPath, + StandardOpenOption.READ, + StandardOpenOption.WRITE, + StandardOpenOption.CREATE); + + // Try to acquire exclusive lock fileLock = channel.tryLock(); if (fileLock != null) { + // Successfully acquired lock - track it for diagnostics + String lockInfo = + String.format( + "%s acquired at %d", + lockAttemptInfo, System.currentTimeMillis()); + activeLocks.put(settingsPath.toString(), lockInfo); + + // Successfully acquired lock - perform the update try { // Validate that we can actually read/write to detect stale locks if (!Files.isWritable(settingsPath)) { @@ -467,15 +508,6 @@ public class GeneralUtils { "Settings file is not writable - permissions issue"); } - // Check for concurrent modifications - String currentHash = calculateSettingsHash(); - if (lastSettingsHash != null && !lastSettingsHash.equals(currentHash)) { - log.info( - "Settings file was modified externally for key: {} - updating hash", - key); - lastSettingsHash = currentHash; - } - // Perform the actual update String[] keyArray = key.split("\\."); YamlHelper settingsYaml = new YamlHelper(settingsPath); @@ -483,52 +515,166 @@ public class GeneralUtils { settingsYaml.saveOverride(settingsPath); // Update hash after successful write - lastSettingsHash = calculateSettingsHash(); + lastSettingsHash = null; // Will be recalculated on next access - log.debug("Successfully updated setting: {} = {}", key, newValue); + log.debug( + "Successfully updated setting: {} = {} (attempt {}) by {}", + key, + newValue, + retryCount + 1, + lockAttemptInfo); return; // Success - exit method } finally { + // Remove from active locks tracking + activeLocks.remove(settingsPath.toString()); + // Ensure file lock is always released - if (fileLock != null && fileLock.isValid()) { - try { - fileLock.release(); - } catch (IOException e) { - log.warn( - "Failed to release file lock for setting {}: {}", - key, - e.getMessage()); - } + if (fileLock.isValid()) { + fileLock.release(); } } } else { - // Lock not available, wait briefly before retry - Thread.sleep(100); + // Lock not available - log diagnostic info + retryCount++; + log.debug( + "File lock not available for setting '{}', attempt {} of {} by {}. Active locks: {}", + key, + retryCount, + maxRetries, + lockAttemptInfo, + activeLocks.isEmpty() ? "none" : activeLocks); + + // Wait before retry with exponential backoff (100ms, 150ms, 200ms, etc.) + long waitTime = Math.min(100 + (retryCount * 50), 500); + Thread.sleep(waitTime); } - } catch (IOException e) { - if (fileLock != null && fileLock.isValid()) { - try { - fileLock.release(); - } catch (IOException releaseError) { - log.warn( - "Failed to release file lock after error: {}", - releaseError.getMessage()); - } + } catch (OverlappingFileLockException e) { + // This specific exception means another thread in the same JVM has the lock + retryCount++; + log.debug( + "Overlapping file lock detected for setting '{}', attempt {} of {} by {}. Another thread in this JVM has the lock. Active locks: {}", + key, + retryCount, + maxRetries, + lockAttemptInfo, + activeLocks); + + try { + // Wait before retry with exponential backoff + long waitTime = Math.min(100 + (retryCount * 50), 500); + Thread.sleep(waitTime); + } catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + throw new IOException("Interrupted while waiting for file lock retry", ie); + } + } catch (IOException e) { + // If this is a specific lock contention error, continue retrying + if (e.getMessage() != null + && (e.getMessage().contains("locked") + || e.getMessage().contains("another process") + || e.getMessage().contains("sharing violation"))) { + + retryCount++; + log.debug( + "File lock contention for setting '{}', attempt {} of {} by {}. IOException: {}. Active locks: {}", + key, + retryCount, + maxRetries, + lockAttemptInfo, + e.getMessage(), + activeLocks); + + try { + // Wait before retry with exponential backoff + long waitTime = Math.min(100 + (retryCount * 50), 500); + Thread.sleep(waitTime); + } catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + throw new IOException( + "Interrupted while waiting for file lock retry", ie); + } + } else { + // Different type of IOException - don't retry + throw e; } - throw e; } catch (InterruptedException e) { Thread.currentThread().interrupt(); throw new IOException("Interrupted while waiting for file lock", e); + } finally { + // Clean up resources + if (fileLock != null && fileLock.isValid()) { + try { + fileLock.release(); + } catch (IOException e) { + log.warn( + "Failed to release file lock for setting {}: {}", + key, + e.getMessage()); + } + } + if (channel != null && channel.isOpen()) { + try { + channel.close(); + } catch (IOException e) { + log.warn( + "Failed to close file channel for setting {}: {}", + key, + e.getMessage()); + } + } } } // If we get here, we couldn't acquire the file lock within timeout + // If no internal locks are active, this is likely an external system lock + // Try one final force write attempt if bypass is enabled + if (FORCE_BYPASS_EXTERNAL_LOCKS && activeLocks.isEmpty()) { + log.debug( + "No internal locks detected - attempting force write bypass for setting '{}' due to external system lock (bypass enabled)", + key); + try { + // Attempt direct file write without locking + String[] keyArray = key.split("\\."); + YamlHelper settingsYaml = new YamlHelper(settingsPath); + settingsYaml.updateValue(Arrays.asList(keyArray), newValue); + settingsYaml.saveOverride(settingsPath); + + // Update hash after successful write + lastSettingsHash = null; + + log.debug( + "Force write bypass successful for setting '{}' = {} (external system lock detected)", + key, + newValue); + return; // Success + + } catch (Exception forceWriteException) { + log.error( + "Force write bypass failed for setting '{}': {}", + key, + forceWriteException.getMessage()); + // Fall through to original exception + } + } else if (!FORCE_BYPASS_EXTERNAL_LOCKS && activeLocks.isEmpty()) { + log.debug( + "External system lock detected for setting '{}' but force bypass is disabled (use -Dstirling.settings.force-bypass-locks=true to enable)", + key); + } + throw new IOException( String.format( - "Could not acquire file lock for setting '%s' within %d ms. " - + "The settings file may be locked by another process or there may be file system issues.", - key, FILE_LOCK_TIMEOUT_MS)); + "Could not acquire file lock for setting '%s' within %d ms by %s. " + + "The settings file may be locked by another process or there may be file system issues. " + + "Final active locks: %s. Force bypass %s", + key, + FILE_LOCK_TIMEOUT_MS, + lockAttemptInfo, + activeLocks, + activeLocks.isEmpty() + ? "attempted but failed" + : "not attempted (internal locks detected)")); } catch (InterruptedException e) { Thread.currentThread().interrupt(); @@ -542,6 +688,17 @@ public class GeneralUtils { } finally { if (lockAcquired) { settingsLock.writeLock().unlock(); + + // Recalculate hash after releasing the write lock + try { + if (lastSettingsHash == null) { + lastSettingsHash = calculateSettingsHash(); + log.debug("Updated settings hash after write operation"); + } + } catch (Exception e) { + log.warn("Failed to update settings hash after write: {}", e.getMessage()); + lastSettingsHash = ""; + } } } } @@ -557,15 +714,133 @@ public class GeneralUtils { return ""; } - byte[] fileBytes = Files.readAllBytes(settingsPath); - MessageDigest md = MessageDigest.getInstance("MD5"); - byte[] hashBytes = md.digest(fileBytes); + boolean lockAcquired = false; + try { + lockAcquired = settingsLock.readLock().tryLock(LOCK_TIMEOUT_SECONDS, TimeUnit.SECONDS); + if (!lockAcquired) { + throw new IOException( + String.format( + "Could not acquire read lock for settings hash calculation within %d seconds. " + + "System may be under heavy load or there may be a deadlock.", + LOCK_TIMEOUT_SECONDS)); + } - StringBuilder sb = new StringBuilder(); - for (byte b : hashBytes) { - sb.append(String.format("%02x", b)); + // Use OS-level file locking with proper retry logic + long startTime = System.currentTimeMillis(); + int retryCount = 0; + final int maxRetries = (int) (FILE_LOCK_TIMEOUT_MS / 100); // 100ms intervals + + while ((System.currentTimeMillis() - startTime) < FILE_LOCK_TIMEOUT_MS) { + FileChannel channel = null; + FileLock fileLock = null; + + try { + // Open channel and keep it open during lock attempts + channel = FileChannel.open(settingsPath, StandardOpenOption.READ); + + // Try to acquire shared lock for reading + fileLock = channel.tryLock(0L, Long.MAX_VALUE, true); + + if (fileLock != null) { + // Successfully acquired lock - calculate hash + try { + byte[] fileBytes = Files.readAllBytes(settingsPath); + MessageDigest md = MessageDigest.getInstance("MD5"); + byte[] hashBytes = md.digest(fileBytes); + + StringBuilder sb = new StringBuilder(); + for (byte b : hashBytes) { + sb.append(String.format("%02x", b)); + } + log.debug( + "Successfully calculated settings hash (attempt {})", + retryCount + 1); + return sb.toString(); + } finally { + fileLock.release(); + } + } else { + // Lock not available - log and retry + retryCount++; + log.debug( + "File lock not available for hash calculation, attempt {} of {}", + retryCount, + maxRetries); + + // Wait before retry with exponential backoff + long waitTime = Math.min(100 + (retryCount * 50), 500); + Thread.sleep(waitTime); + } + + } catch (IOException e) { + // If this is a specific lock contention error, continue retrying + if (e.getMessage() != null + && (e.getMessage().contains("locked") + || e.getMessage().contains("another process") + || e.getMessage().contains("sharing violation"))) { + + retryCount++; + log.debug( + "File lock contention for hash calculation, attempt {} of {}: {}", + retryCount, + maxRetries, + e.getMessage()); + + try { + // Wait before retry with exponential backoff + long waitTime = Math.min(100 + (retryCount * 50), 500); + Thread.sleep(waitTime); + } catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + throw new IOException( + "Interrupted while waiting for file lock retry", ie); + } + } else { + // Different type of IOException - don't retry + throw e; + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new IOException("Interrupted while waiting for file lock", e); + } finally { + // Clean up resources + if (fileLock != null && fileLock.isValid()) { + try { + fileLock.release(); + } catch (IOException e) { + log.warn( + "Failed to release file lock for hash calculation: {}", + e.getMessage()); + } + } + if (channel != null && channel.isOpen()) { + try { + channel.close(); + } catch (IOException e) { + log.warn( + "Failed to close file channel for hash calculation: {}", + e.getMessage()); + } + } + } + } + + // If we couldn't get the file lock, throw an exception + throw new IOException( + String.format( + "Could not acquire file lock for settings hash calculation within %d ms. " + + "The settings file may be locked by another process.", + FILE_LOCK_TIMEOUT_MS)); + + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new IOException( + "Interrupted while waiting for settings read lock for hash calculation", e); + } finally { + if (lockAcquired) { + settingsLock.readLock().unlock(); + } } - return sb.toString(); } /** diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java index 50c816ea8..cec30f535 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java @@ -1,6 +1,9 @@ package stirling.software.proprietary.security.controller.api; import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.Map; import java.util.regex.Pattern; @@ -27,6 +30,7 @@ import jakarta.validation.Valid; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; +import stirling.software.common.configuration.InstallationPathConfig; import stirling.software.common.model.ApplicationProperties; import stirling.software.common.util.GeneralUtils; import stirling.software.proprietary.security.model.api.admin.SettingValueResponse; @@ -60,6 +64,48 @@ public class AdminSettingsController { return ResponseEntity.ok(applicationProperties); } + @GetMapping("/file") + @Operation( + summary = "Get settings file content", + description = + "Retrieve the raw settings.yml file content showing the latest saved values (after restart). Admin access required.") + @ApiResponses( + value = { + @ApiResponse( + responseCode = "200", + description = "Settings file retrieved successfully"), + @ApiResponse(responseCode = "404", description = "Settings file not found"), + @ApiResponse( + responseCode = "403", + description = "Access denied - Admin role required"), + @ApiResponse(responseCode = "500", description = "Failed to read settings file") + }) + public ResponseEntity getSettingsFile() { + try { + Path settingsPath = Paths.get(InstallationPathConfig.getSettingsPath()); + if (!Files.exists(settingsPath)) { + return ResponseEntity.notFound().build(); + } + + String fileContent = Files.readString(settingsPath); + log.debug("Admin requested settings file content"); + + // Return as JSON with the file content + Map response = + Map.of("filePath", settingsPath.toString(), "content", fileContent); + return ResponseEntity.ok(response); + + } catch (IOException e) { + log.error("Failed to read settings file: {}", e.getMessage(), e); + return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) + .body("Failed to read settings file: " + e.getMessage()); + } catch (Exception e) { + log.error("Unexpected error reading settings file: {}", e.getMessage(), e); + return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) + .body("Unexpected error reading settings file"); + } + } + @PutMapping @Operation( summary = "Update application settings (delta updates)", From f9d36b985ae39796b69193fa1eec5f5298bb31f7 Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+Frooodle@users.noreply.github.com.> Date: Mon, 28 Jul 2025 11:58:10 +0100 Subject: [PATCH 12/13] change file to be json, added delta endpoint --- .../api/AdminSettingsController.java | 137 ++++++++++++++++-- 1 file changed, 125 insertions(+), 12 deletions(-) diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java index cec30f535..452323526 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java @@ -4,6 +4,7 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.HashMap; import java.util.Map; import java.util.regex.Pattern; @@ -19,6 +20,7 @@ import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.util.HtmlUtils; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; import io.swagger.v3.oas.annotations.Operation; import io.swagger.v3.oas.annotations.responses.ApiResponse; @@ -66,19 +68,21 @@ public class AdminSettingsController { @GetMapping("/file") @Operation( - summary = "Get settings file content", + summary = "Get settings file as JSON", description = - "Retrieve the raw settings.yml file content showing the latest saved values (after restart). Admin access required.") + "Retrieve the settings.yml file parsed as JSON, showing the latest saved values (after restart). Comments and formatting are ignored. Admin access required.") @ApiResponses( value = { @ApiResponse( responseCode = "200", - description = "Settings file retrieved successfully"), + description = "Settings file retrieved and parsed successfully"), @ApiResponse(responseCode = "404", description = "Settings file not found"), @ApiResponse( responseCode = "403", description = "Access denied - Admin role required"), - @ApiResponse(responseCode = "500", description = "Failed to read settings file") + @ApiResponse( + responseCode = "500", + description = "Failed to read or parse settings file") }) public ResponseEntity getSettingsFile() { try { @@ -87,18 +91,19 @@ public class AdminSettingsController { return ResponseEntity.notFound().build(); } - String fileContent = Files.readString(settingsPath); - log.debug("Admin requested settings file content"); + // Parse YAML file to JSON + ObjectMapper yamlMapper = new ObjectMapper(new YAMLFactory()); + ObjectMapper jsonMapper = new ObjectMapper(); - // Return as JSON with the file content - Map response = - Map.of("filePath", settingsPath.toString(), "content", fileContent); - return ResponseEntity.ok(response); + Object yamlData = yamlMapper.readValue(settingsPath.toFile(), Object.class); + + log.debug("Admin requested settings file as JSON"); + return ResponseEntity.ok(yamlData); } catch (IOException e) { - log.error("Failed to read settings file: {}", e.getMessage(), e); + log.error("Failed to read or parse settings file: {}", e.getMessage(), e); return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) - .body("Failed to read settings file: " + e.getMessage()); + .body("Failed to read or parse settings file: " + e.getMessage()); } catch (Exception e) { log.error("Unexpected error reading settings file: {}", e.getMessage(), e); return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) @@ -106,6 +111,68 @@ public class AdminSettingsController { } } + @GetMapping("/delta") + @Operation( + summary = "Get settings delta (pending changes)", + description = + "Compare current runtime settings with saved file settings to show pending changes that will take effect after restart. Admin access required.") + @ApiResponses( + value = { + @ApiResponse( + responseCode = "200", + description = "Settings delta retrieved successfully"), + @ApiResponse(responseCode = "404", description = "Settings file not found"), + @ApiResponse( + responseCode = "403", + description = "Access denied - Admin role required"), + @ApiResponse( + responseCode = "500", + description = "Failed to calculate settings delta") + }) + public ResponseEntity getSettingsDelta() { + try { + Path settingsPath = Paths.get(InstallationPathConfig.getSettingsPath()); + if (!Files.exists(settingsPath)) { + return ResponseEntity.notFound().build(); + } + + // Get current runtime settings as JSON + Map runtimeSettings = + objectMapper.convertValue(applicationProperties, Map.class); + + // Parse YAML file to get saved settings + ObjectMapper yamlMapper = new ObjectMapper(new YAMLFactory()); + Object fileData = yamlMapper.readValue(settingsPath.toFile(), Object.class); + Map savedSettings = objectMapper.convertValue(fileData, Map.class); + + // Calculate differences + Map delta = new HashMap<>(); + Map pendingChanges = new HashMap<>(); + Map currentValues = new HashMap<>(); + + findDifferences("", runtimeSettings, savedSettings, pendingChanges, currentValues); + + delta.put( + "pendingChanges", pendingChanges); // Values that will take effect after restart + delta.put("currentValues", currentValues); // Current runtime values + delta.put("hasPendingChanges", !pendingChanges.isEmpty()); + + log.debug( + "Admin requested settings delta - found {} pending changes", + pendingChanges.size()); + return ResponseEntity.ok(delta); + + } catch (IOException e) { + log.error("Failed to calculate settings delta: {}", e.getMessage(), e); + return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) + .body("Failed to calculate settings delta: " + e.getMessage()); + } catch (Exception e) { + log.error("Unexpected error calculating settings delta: {}", e.getMessage(), e); + return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) + .body("Unexpected error calculating settings delta"); + } + } + @PutMapping @Operation( summary = "Update application settings (delta updates)", @@ -489,4 +556,50 @@ public class AdminSettingsController { throw new NoSuchFieldException("Property not accessible: " + propertyPath); } } + + /** Recursively compare two maps to find differences between runtime and saved settings */ + @SuppressWarnings("unchecked") + private void findDifferences( + String keyPrefix, + Map runtime, + Map saved, + Map pendingChanges, + Map currentValues) { + + // Check all keys in saved settings (these are the pending changes) + for (Map.Entry entry : saved.entrySet()) { + String key = entry.getKey(); + String fullKey = keyPrefix.isEmpty() ? key : keyPrefix + "." + key; + Object savedValue = entry.getValue(); + Object runtimeValue = runtime.get(key); + + if (savedValue instanceof Map && runtimeValue instanceof Map) { + // Recursively check nested objects + findDifferences( + fullKey, + (Map) runtimeValue, + (Map) savedValue, + pendingChanges, + currentValues); + } else { + // Compare values - if they're different, savedValue is pending + if (!java.util.Objects.equals(runtimeValue, savedValue)) { + pendingChanges.put(fullKey, savedValue); + currentValues.put(fullKey, runtimeValue); + } + } + } + + // Check for keys that exist in runtime but not in saved (these would be removed on restart) + for (Map.Entry entry : runtime.entrySet()) { + String key = entry.getKey(); + String fullKey = keyPrefix.isEmpty() ? key : keyPrefix + "." + key; + + if (!saved.containsKey(key)) { + // This runtime setting would be lost on restart + pendingChanges.put(fullKey + " (will be removed)", null); + currentValues.put(fullKey, entry.getValue()); + } + } + } } From 26a6534ae4bd99bd4392e3cacd503a8aa5ca8180 Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+Frooodle@users.noreply.github.com.> Date: Mon, 28 Jul 2025 12:18:19 +0100 Subject: [PATCH 13/13] remove file endpoint, show delta cahgned to tracking instead --- .../api/AdminSettingsController.java | 188 ++++-------------- 1 file changed, 41 insertions(+), 147 deletions(-) diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java index 452323526..8fc684bbe 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminSettingsController.java @@ -1,11 +1,9 @@ package stirling.software.proprietary.security.controller.api; import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.Paths; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import java.util.regex.Pattern; import org.springframework.http.HttpStatus; @@ -17,10 +15,10 @@ import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.PutMapping; import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.util.HtmlUtils; import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; import io.swagger.v3.oas.annotations.Operation; import io.swagger.v3.oas.annotations.responses.ApiResponse; @@ -32,7 +30,6 @@ import jakarta.validation.Valid; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; -import stirling.software.common.configuration.InstallationPathConfig; import stirling.software.common.model.ApplicationProperties; import stirling.software.common.util.GeneralUtils; import stirling.software.proprietary.security.model.api.admin.SettingValueResponse; @@ -49,11 +46,14 @@ public class AdminSettingsController { private final ApplicationProperties applicationProperties; private final ObjectMapper objectMapper; + + // Track settings that have been modified but not yet applied (require restart) + private static final ConcurrentHashMap pendingChanges = new ConcurrentHashMap<>(); @GetMapping @Operation( summary = "Get all application settings", - description = "Retrieve all current application settings. Admin access required.") + description = "Retrieve all current application settings. Use includePending=true to include settings that will take effect after restart. Admin access required.") @ApiResponses( value = { @ApiResponse(responseCode = "200", description = "Settings retrieved successfully"), @@ -61,116 +61,44 @@ public class AdminSettingsController { responseCode = "403", description = "Access denied - Admin role required") }) - public ResponseEntity getSettings() { - log.debug("Admin requested all application settings"); - return ResponseEntity.ok(applicationProperties); - } - - @GetMapping("/file") - @Operation( - summary = "Get settings file as JSON", - description = - "Retrieve the settings.yml file parsed as JSON, showing the latest saved values (after restart). Comments and formatting are ignored. Admin access required.") - @ApiResponses( - value = { - @ApiResponse( - responseCode = "200", - description = "Settings file retrieved and parsed successfully"), - @ApiResponse(responseCode = "404", description = "Settings file not found"), - @ApiResponse( - responseCode = "403", - description = "Access denied - Admin role required"), - @ApiResponse( - responseCode = "500", - description = "Failed to read or parse settings file") - }) - public ResponseEntity getSettingsFile() { - try { - Path settingsPath = Paths.get(InstallationPathConfig.getSettingsPath()); - if (!Files.exists(settingsPath)) { - return ResponseEntity.notFound().build(); - } - - // Parse YAML file to JSON - ObjectMapper yamlMapper = new ObjectMapper(new YAMLFactory()); - ObjectMapper jsonMapper = new ObjectMapper(); - - Object yamlData = yamlMapper.readValue(settingsPath.toFile(), Object.class); - - log.debug("Admin requested settings file as JSON"); - return ResponseEntity.ok(yamlData); - - } catch (IOException e) { - log.error("Failed to read or parse settings file: {}", e.getMessage(), e); - return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) - .body("Failed to read or parse settings file: " + e.getMessage()); - } catch (Exception e) { - log.error("Unexpected error reading settings file: {}", e.getMessage(), e); - return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) - .body("Unexpected error reading settings file"); + public ResponseEntity getSettings(@RequestParam(defaultValue = "false") boolean includePending) { + log.debug("Admin requested all application settings (includePending={})", includePending); + + if (!includePending) { + return ResponseEntity.ok(applicationProperties); } + + // Include pending changes in response + Map response = new HashMap<>(); + response.put("currentSettings", applicationProperties); + response.put("pendingChanges", pendingChanges); + response.put("hasPendingChanges", !pendingChanges.isEmpty()); + + return ResponseEntity.ok(response); } @GetMapping("/delta") @Operation( - summary = "Get settings delta (pending changes)", + summary = "Get pending settings changes", description = - "Compare current runtime settings with saved file settings to show pending changes that will take effect after restart. Admin access required.") + "Retrieve settings that have been modified but not yet applied (require restart). Admin access required.") @ApiResponses( value = { @ApiResponse( responseCode = "200", - description = "Settings delta retrieved successfully"), - @ApiResponse(responseCode = "404", description = "Settings file not found"), + description = "Pending changes retrieved successfully"), @ApiResponse( responseCode = "403", - description = "Access denied - Admin role required"), - @ApiResponse( - responseCode = "500", - description = "Failed to calculate settings delta") + description = "Access denied - Admin role required") }) public ResponseEntity getSettingsDelta() { - try { - Path settingsPath = Paths.get(InstallationPathConfig.getSettingsPath()); - if (!Files.exists(settingsPath)) { - return ResponseEntity.notFound().build(); - } - - // Get current runtime settings as JSON - Map runtimeSettings = - objectMapper.convertValue(applicationProperties, Map.class); - - // Parse YAML file to get saved settings - ObjectMapper yamlMapper = new ObjectMapper(new YAMLFactory()); - Object fileData = yamlMapper.readValue(settingsPath.toFile(), Object.class); - Map savedSettings = objectMapper.convertValue(fileData, Map.class); - - // Calculate differences - Map delta = new HashMap<>(); - Map pendingChanges = new HashMap<>(); - Map currentValues = new HashMap<>(); - - findDifferences("", runtimeSettings, savedSettings, pendingChanges, currentValues); - - delta.put( - "pendingChanges", pendingChanges); // Values that will take effect after restart - delta.put("currentValues", currentValues); // Current runtime values - delta.put("hasPendingChanges", !pendingChanges.isEmpty()); - - log.debug( - "Admin requested settings delta - found {} pending changes", - pendingChanges.size()); - return ResponseEntity.ok(delta); - - } catch (IOException e) { - log.error("Failed to calculate settings delta: {}", e.getMessage(), e); - return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) - .body("Failed to calculate settings delta: " + e.getMessage()); - } catch (Exception e) { - log.error("Unexpected error calculating settings delta: {}", e.getMessage(), e); - return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) - .body("Unexpected error calculating settings delta"); - } + Map response = new HashMap<>(); + response.put("pendingChanges", pendingChanges); + response.put("hasPendingChanges", !pendingChanges.isEmpty()); + response.put("count", pendingChanges.size()); + + log.debug("Admin requested pending changes - found {} settings", pendingChanges.size()); + return ResponseEntity.ok(response); } @PutMapping @@ -209,6 +137,10 @@ public class AdminSettingsController { log.info("Admin updating setting: {} = {}", key, value); GeneralUtils.saveKeyToSettings(key, value); + + // Track this as a pending change + pendingChanges.put(key, value); + updatedCount++; } @@ -314,6 +246,10 @@ public class AdminSettingsController { log.info("Admin updating section setting: {} = {}", fullKey, value); GeneralUtils.saveKeyToSettings(fullKey, value); + + // Track this as a pending change + pendingChanges.put(fullKey, value); + updatedCount++; } @@ -401,6 +337,9 @@ public class AdminSettingsController { Object value = request.getValue(); log.info("Admin updating single setting: {} = {}", key, value); GeneralUtils.saveKeyToSettings(key, value); + + // Track this as a pending change + pendingChanges.put(key, value); String escapedKey = HtmlUtils.htmlEscape(key); return ResponseEntity.ok( @@ -557,49 +496,4 @@ public class AdminSettingsController { } } - /** Recursively compare two maps to find differences between runtime and saved settings */ - @SuppressWarnings("unchecked") - private void findDifferences( - String keyPrefix, - Map runtime, - Map saved, - Map pendingChanges, - Map currentValues) { - - // Check all keys in saved settings (these are the pending changes) - for (Map.Entry entry : saved.entrySet()) { - String key = entry.getKey(); - String fullKey = keyPrefix.isEmpty() ? key : keyPrefix + "." + key; - Object savedValue = entry.getValue(); - Object runtimeValue = runtime.get(key); - - if (savedValue instanceof Map && runtimeValue instanceof Map) { - // Recursively check nested objects - findDifferences( - fullKey, - (Map) runtimeValue, - (Map) savedValue, - pendingChanges, - currentValues); - } else { - // Compare values - if they're different, savedValue is pending - if (!java.util.Objects.equals(runtimeValue, savedValue)) { - pendingChanges.put(fullKey, savedValue); - currentValues.put(fullKey, runtimeValue); - } - } - } - - // Check for keys that exist in runtime but not in saved (these would be removed on restart) - for (Map.Entry entry : runtime.entrySet()) { - String key = entry.getKey(); - String fullKey = keyPrefix.isEmpty() ? key : keyPrefix + "." + key; - - if (!saved.containsKey(key)) { - // This runtime setting would be lost on restart - pendingChanges.put(fullKey + " (will be removed)", null); - currentValues.put(fullKey, entry.getValue()); - } - } - } }