Harden shared signature endpoints (#5806)

This commit is contained in:
Anthony Stirling
2026-02-26 12:53:47 +00:00
committed by GitHub
parent b21e1313d8
commit 1bac8417af
3 changed files with 111 additions and 0 deletions

View File

@@ -48,11 +48,19 @@ public class SignatureController {
* requirements.
*/
@PostMapping
@PreAuthorize("isAuthenticated() && !hasAuthority('ROLE_DEMO_USER')")
public ResponseEntity<SavedSignatureResponse> saveSignature(
@RequestBody SavedSignatureRequest request) {
try {
String username = userService.getCurrentUsername();
if ("shared".equals(request.getScope()) && !userService.isCurrentUserAdmin()) {
log.warn(
"User {} attempted to create shared signature without admin role",
username);
return ResponseEntity.status(HttpStatus.FORBIDDEN).build();
}
// Validate request
if (request.getDataUrl() == null || request.getDataUrl().isEmpty()) {
log.warn("User {} attempted to save signature without dataUrl", username);
@@ -76,6 +84,7 @@ public class SignatureController {
* signatures.
*/
@GetMapping
@PreAuthorize("isAuthenticated() && !hasAuthority('ROLE_DEMO_USER')")
public ResponseEntity<List<SavedSignatureResponse>> listSignatures() {
try {
String username = userService.getCurrentUsername();
@@ -98,12 +107,21 @@ public class SignatureController {
try {
String username = userService.getCurrentUsername();
String newLabel = body.get("label");
boolean isAdmin = userService.isCurrentUserAdmin();
if (newLabel == null || newLabel.trim().isEmpty()) {
log.warn("Invalid label update request");
return ResponseEntity.badRequest().build();
}
if (signatureService.isSharedSignature(signatureId) && !isAdmin) {
log.warn(
"User {} attempted to update shared signature {} without admin role",
username,
signatureId);
return ResponseEntity.status(HttpStatus.FORBIDDEN).build();
}
signatureService.updateSignatureLabel(username, signatureId, newLabel);
log.info("User {} updated label for signature {}", username, signatureId);
return ResponseEntity.noContent().build();

View File

@@ -249,6 +249,12 @@ public class SignatureService implements PersonalSignatureServiceInterface {
throw new FileNotFoundException("Signature metadata not found");
}
public boolean isSharedSignature(String signatureId) {
validateFileName(signatureId);
Path sharedFolder = Paths.get(SIGNATURE_BASE_PATH, ALL_USERS_FOLDER);
return Files.exists(sharedFolder.resolve(signatureId + ".json"));
}
private void updateMetadataLabel(Path metadataPath, String newLabel) throws IOException {
String metadataJson = Files.readString(metadataPath, StandardCharsets.UTF_8);
SavedSignatureResponse sig =

View File

@@ -0,0 +1,87 @@
package stirling.software.proprietary.controller.api;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.http.MediaType;
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.test.web.servlet.setup.MockMvcBuilders;
import stirling.software.proprietary.security.service.UserService;
import stirling.software.proprietary.service.SignatureService;
@ExtendWith(MockitoExtension.class)
class SignatureControllerTest {
@Mock private SignatureService signatureService;
@Mock private UserService userService;
private MockMvc mockMvc;
@BeforeEach
void setUp() {
SignatureController controller = new SignatureController(signatureService, userService);
mockMvc = MockMvcBuilders.standaloneSetup(controller).build();
}
@Test
void saveSignatureForbidsSharedScopeForNonAdmin() throws Exception {
when(userService.getCurrentUsername()).thenReturn("user1");
when(userService.isCurrentUserAdmin()).thenReturn(false);
mockMvc.perform(
post("/api/v1/proprietary/signatures")
.contentType(MediaType.APPLICATION_JSON)
.content(
"""
{
"id": "sig1",
"scope": "shared",
"dataUrl": "data:image/png;base64,AAAA"
}
"""))
.andExpect(status().isForbidden());
verify(signatureService, never()).saveSignature(any(), any());
}
@Test
void updateSignatureLabelForbidsSharedSignatureForNonAdmin() throws Exception {
when(userService.getCurrentUsername()).thenReturn("user1");
when(userService.isCurrentUserAdmin()).thenReturn(false);
when(signatureService.isSharedSignature("sig123")).thenReturn(true);
mockMvc.perform(
post("/api/v1/proprietary/signatures/sig123/label")
.contentType(MediaType.APPLICATION_JSON)
.content("{\"label\":\"new label\"}"))
.andExpect(status().isForbidden());
verify(signatureService, never()).updateSignatureLabel(any(), any(), any());
}
@Test
void updateSignatureLabelAllowsPersonalSignatureForNonAdmin() throws Exception {
when(userService.getCurrentUsername()).thenReturn("user1");
when(userService.isCurrentUserAdmin()).thenReturn(false);
when(signatureService.isSharedSignature("sig123")).thenReturn(false);
mockMvc.perform(
post("/api/v1/proprietary/signatures/sig123/label")
.contentType(MediaType.APPLICATION_JSON)
.content("{\"label\":\"new label\"}"))
.andExpect(status().isNoContent());
verify(signatureService).updateSignatureLabel(eq("user1"), eq("sig123"), eq("new label"));
}
}